Re: How to configure DH groups for TLS 1.3

[Lukas Tribus]

On Thu, 2 May 2024 at 19:50, Lukas Tribus  wrote:
>
> On Thu, 2 May 2024 at 17:14, Froehlich, Dominik
>  wrote:
> > The closest I’ve gotten is the “curves” property: 
> > https://docs.haproxy.org/2.8/configuration.html#5.1-curves
> >
> > However, I think it only restricts the available elliptic curves in a ECDHE 
> > handshake, but it does not prevent a TLS 1.3 client from selecting a 
> > non-ECDHE prime group, for example “ffdhe8192”.
>
> If I understand the code correctly, both nginx and haproxy call
> SSL_CTX_set1_curves_list(), what exactly makes you think that haproxy
> does something different?

More to the point:

curve and group is the same exact thing in openssl:


https://www.openssl.org/docs/man3.0/man3/SSL_CONF_cmd.html

> -curves groups
> This is a synonym for the -groups command.


https://www.openssl.org/docs/man3.0/man3/SSL_CTX_set1_curves.html

> The curve functions are synonyms for the equivalently named group functions 
> and are identical in every respect. They exist because, prior to TLS1.3, 
> there was only the concept of supported curves. In TLS1.3 this was renamed to 
> supported groups, and extended to include Diffie Hellman groups. The group 
> functions should be used in preference.


https://github.com/openssl/openssl/issues/18089#issuecomment-1096748557

> In TLSv1.3 the old "supported_curves" extension was renamed to 
> "supported_groups". This renaming has been followed through to the OpenSSL 
> API so that SSL_CTX_set1_curves_list is synonymous with 
> SSL_CTX_set1_groups_list, and the the -curves command line argument is 
> synonymous with -groups. So in the above issue you are not just constraining 
> the EC curves - you are constraining all the groups available for use in 
> TLSv1.3. This includes FFDH groups - so the above configuration prevents 
> either ECDH or FFDH being used in TLSv1.3.


Setting openssl curves (groups) via SSL_CTX_set1_curves_list just like
nginx does is supported since Haproxy 1.8:

https://github.com/haproxy/haproxy/commit/e7f2b7301c0a6625654056356cca56853a14cd68


Lukas

Re: [PATCH 0/2] CI fixes, spelling cleanup

[Willy Tarreau]

On Tue, Apr 30, 2024 at 04:11:25PM +0200, Ilia Shipitsin wrote:
> NetBSD image was updated to 10.0, pcre2 is available out
> of box now
(...)

Both merged now, thank you Ilya!
Willy

Re: How to configure DH groups for TLS 1.3

[Lukas Tribus]

On Thu, 2 May 2024 at 17:14, Froehlich, Dominik
 wrote:
> The closest I’ve gotten is the “curves” property: 
> https://docs.haproxy.org/2.8/configuration.html#5.1-curves
>
> However, I think it only restricts the available elliptic curves in a ECDHE 
> handshake, but it does not prevent a TLS 1.3 client from selecting a 
> non-ECDHE prime group, for example “ffdhe8192”.

If I understand the code correctly, both nginx and haproxy call
SSL_CTX_set1_curves_list(), what exactly makes you think that haproxy
does something different?


Lukas

Re: maxconn definition in frontend or backend section ?

[Lukas Tribus]

On Thu, 2 May 2024 at 15:22, Roberto Carna  wrote:
>
> Dear all, I have HAproxy in front of a web server node.
>
> I want the web server node to accept just 1000 concurrent connections.
>
> So I want to use the maxconn parameter in order to let new connections
> above 1000 to wait until the web service has free workers.
>
> According to what I read, if I define maxconn in frontend section of
> haproxy.cfg, the incoming connections above 1000 will wait in the
> kernel socket queue, and if I define the parameter in the backend
> section the connections above 1000 will wait in the web server node
> until there are workers free.
>
> So where is the best section to define the maxconn parameter???

If you want to limit the connections to a server, that's is exactly
where you put the limit: *server* maxconn
There is no "backend" maxconn, it's always defined per server.

Leave more room in the frontend, e.g. 2000 or 3000 and set global
maxconn way above the total amount of connections (considering both
front and backend/server connections (like 1 in this example).

This way, haproxy handles the queuing. If you leave that up to the
kernel, haproxy will not see the connections above this threshold at
all. No logging, no stats, and you may even loose access to the
haproxy stats interface, if it is on the same frontend.


Lukas

Re: How to configure DH groups for TLS 1.3

[Илья Шипицин]

I'd try openssl.cnf

чт, 2 мая 2024 г. в 17:17, Froehlich, Dominik :

> Hello everyone,
>
>
>
> I’m hardening HAProxy for CVE-2002-20001 (DHEAT attack) at the moment.
>
>
>
> For TLS 1.2 I’m using the “tune.ssl.default-dh-param” option to limit the
> key size to 2048 bit so that an attacker can’t force huge keys and thus
> lots of CPU cycles on the server.
>
>
>
> However, I’ve noticed that the property has no effect on TLS 1.3
> connections. An attacker can still negotiate an 8192-bit key and brick the
> server with relative ease.
>
>
>
> I’ve found an OpenSSL blog article about the issue:
> https://www.openssl.org/blog/blog/2022/10/21/tls-groups-configuration/index.html
>
>
>
> As it seems, this used to be a non-issue with OpenSSL 1.1.1 because it
> only supported EC groups, not finite field ones but in OpenSSL 3.x it is
> again possible to select the vulnerable groups, even with TLS 1.3.
>
>
>
> The article mentions a way of configuring OpenSSL with a “Groups” setting
> to restrict the number of supported DH groups, however I haven’t found any
> HAProxy config option equivalent.
>
>
>
> The closest I’ve gotten is the “curves” property:
> https://docs.haproxy.org/2.8/configuration.html#5.1-curves
>
>
>
> However, I think it only restricts the available elliptic curves in a
> ECDHE handshake, but it does not prevent a TLS 1.3 client from selecting a
> non-ECDHE prime group, for example “ffdhe8192”.
>
>
>
> The article provides example configurations for NGINX and Apache, but is
> there any way to restrict the DH groups (e.g to just ECDHE) for TLS 1.3 for
> HAProxy, too?
>
>
>
>
>
> Best Regards,
>
> Dominik
>

RE: Updated list RSA Conference 2024

[Bonny Rodger]

Hi,

I can forward the pricing and other details for your consideration.

awaiting for positive response.
Bonny Rodger

From: Bonny Rodger
Sent: Monday, April 22, 2024 4:37 PM
To: haproxy@formilux.org
Subject: Updated list RSA Conference 2024

Hi,

Recently updated Attendees contacts of RSA Conference 2024 with Emails.

If you're interested, I'm happy to send you total numbers and pricing details 
for your review.

Professionals profile:-  IT security, Software/hardware, Executives and 
Management, Government Officials and Policymakers, Researchers and Academics, 
cybersecurity analysts and engineers.

Awaiting Response,
Bonny Rodger - Event coordinator

Re: Article submission for haproxy.org

[Raddie Kalytenko]

Hi there,
Hope all is well!

I'm following up on my previous email.
Just wondering if you received it.
Please let me know if you are interested in a new article for your website.

Cheers,
*Raddie Kalytenko*


On Thu, Apr 25, 2024 at 5:45 PM Raddie Kalytenko 
wrote:

> Hi there,
> I hope you are doing well!
>
> My name is Raddie. I am contacting you on behalf of lawrina.org.
> This is our legaltech space with professionally designed legal templates,
> guides, and AI-driven software for contract drafting.
>
> I was searching for new websites to collaborate with and came across
> yours.
> Need to say that you did good work! The website is great :)
> I am interested in submitting a post for haproxy.org.
>
> We will create content that will be unique, special for your platform, and
> useful for your audience.
> In return, I need a backlink to our site in this article.
> I am confident that our article will provide your audience with valuable
> legal knowledge and increase your website traffic.
>
> The article can have a custom topic, about tech laws, for example.
> Please let me know if you are interested.
> I will be happy to provide you with more details: topics, prices, terms,
> and other conditions.
>
> Thank you for considering this opportunity!
> Best regards,
>
> *Raddie Kalytenko*
>
>

Re: Question on deleting cookies from an HTTP request

[Willy Tarreau]

Hi,

On Sat, Apr 27, 2024 at 02:06:54AM +0200, Aleksandar Lazic wrote:
> Hi Lokesh.
> 
> On 2024-04-27 (Sa.) 01:41, Lokesh Jindal wrote:
> > Hey folks
> > 
> > I have found that there is no operator "del-cookie" in HAProxy to delete
> > cookies from the request. (HAProxy does support the operator
> > "del-header").
> > 
> > Can you explain why such an operator is not supported? Is it due to
> > complexity? Due to performance? It will be great if you can share
> > details behind this design choice.
> 
> Well I'm pretty sure because nobody have added this feature into HAProxy.
> You are welcome to send a patch which add this feature.
> 
> Maybe you could add "delete" into the
> https://docs.haproxy.org/2.9/configuration.html#4.2-cookie function.
> 
> Please take a look into
> https://github.com/haproxy/haproxy/blob/master/CONTRIBUTING file if you plan
> to contribute.
> 
> > We have use cases where we want to delete cookies from the request. Not
> > having this support in HAProxy also makes me question if one should be
> > deleting request cookies in the reverse proxy layer.
> 
> Maybe you can use some of the "*-header" functions to remove the cookie as
> shown in the example in
> https://docs.haproxy.org/2.9/configuration.html#4.4-replace-header

Lukas had already provided some fairly complete info on how to do it here:

   https://discourse.haproxy.org/t/best-way-to-delete-a-cookie/3184

Since then we've got the "replace-value" action that does the job for
comma-delimited values, but sadly there's still this bogus syntax in the
Cookie header where a semi-colon is used as the delimiter so replace-value
cannot be used there.

Requests for cookie removal are very rare but have always been present.
I'm really wondering if we should implement a specific action for this
instead of relying on replace-header rules. If adding 2-3 rules for
these rare cases is not considered something too painful to maintain,
I'd prefer it remains that way. If it comes at a cost (e.g. regex match)
then maybe we'll need to think about it for 3.1.

Regards,
Willy

Re: Question on deleting cookies from an HTTP request

[Aleksandar Lazic]

Hi Lokesh.

On 2024-04-27 (Sa.) 01:41, Lokesh Jindal wrote:

Hey folks

I have found that there is no operator "del-cookie" in HAProxy to delete cookies 
from the request. (HAProxy does support the operator "del-header").


Can you explain why such an operator is not supported? Is it due to complexity? 
Due to performance? It will be great if you can share details behind this design 
choice.


Well I'm pretty sure because nobody have added this feature into HAProxy. You 
are welcome to send a patch which add this feature.


Maybe you could add "delete" into the 
https://docs.haproxy.org/2.9/configuration.html#4.2-cookie function.


Please take a look into 
https://github.com/haproxy/haproxy/blob/master/CONTRIBUTING file if you plan to 
contribute.


We have use cases where we want to delete cookies from the request. Not having 
this support in HAProxy also makes me question if one should be deleting request 
cookies in the reverse proxy layer.


Maybe you can use some of the "*-header" functions to remove the cookie as shown 
in the example in https://docs.haproxy.org/2.9/configuration.html#4.4-replace-header



Thanks
Lokesh


Regards
Alex

Re: Odd warnings when using "option forwarded"

[Aurelien DARRAGON]

> That's interesting, because I already had `option  forwardfor except 
> 127.0.0.1` in the frontend which works perfectly.  Should that be in the 
> backend too? 

No, it's OK to use forwardfor in frontend sections since it is
historical behavior. But forwarded doesn't allow that and requires
explicit backend configuration.

> I was trying to find 'option forwarded' in the documentation, but I couldn't 
> click on it after finding it with the search box and I couldn't search all 
> the documentation with the browser, as it wasn't all on one page.

Here is the relevant config section:
https://docs.haproxy.org/dev/configuration.html#4.2-option%20forwarded

Aurelien

Re: Odd warnings when using "option forwarded"

[Shawn Heisey]

On 4/26/24 10:51, Aurelien DARRAGON wrote:

This is expected because forwarded cannot be used on frontend unlike
forwardfor:


That's interesting, because I already had `option  forwardfor except 
127.0.0.1` in the frontend which works perfectly.  Should that be in the 
backend too?


I was trying to find 'option forwarded' in the documentation, but I 
couldn't click on it after finding it with the search box and I couldn't 
search all the documentation with the browser, as it wasn't all on one page.


Thanks,
Shawn

Re: Odd warnings when using "option forwarded"

[Aurelien DARRAGON]

Hi Shawn

On 26/04/2024 18:34, Shawn Heisey wrote:
> I was just reading about the new "option forwarded" capability in 2.9,
> so I added it to my config and now I get warnings when checking the config.
> 
> [WARNING]  (253408) : config : parsing [/etc/haproxy/haproxy.cfg:45] :
> 'option forwarded' ignored because frontend 'web80' has no backend
> capability.
> [WARNING]  (253408) : config : parsing [/etc/haproxy/haproxy.cfg:60] :
> 'option forwarded' ignored because frontend 'web' has no backend
> capability.
> 
> This is the line I added to the frontends:
> 
>     option  forwarded except 127.0.0.1
> 
> I realized I don't need it on 'web80', so I removed that one.
> 
> The 'web' frontend does use backends.  Its default backend has no
> servers and denies all requests -- the request must match one of the
> ACLs to choose a backend that actually does something.
> 
> I suspect that I can ignore this warning, but I want to check here and
> make sure.  Can I set something so it doesn't emit the warning?
> 
> Thanks,
> Shawn
> 
> 

This is expected because forwarded cannot be used on frontends unlike
the good old forwardfor:

This is expected because forwarded cannot be used on frontend unlike
forwardfor:

> -- keyword -- defaults - frontend - listen -- backend 
> -
> option forwardfor X  X X X
> option forwarded (*)  X  - X X

(excerpt from the documentation)

This was done on purpose because in practice the option is only relevant
at the backend side, and supporting forwardfor in both frontend and
backend config sections has caused multiple issues in the past because
of the added complexity (requires extra care to check for the option on
the proper section, depending if it's set in the frontend or the backend
or both...), and we wanted to get rid of that extra complexity from the
start when implementing the new forwarded option.

Thus for the option to be considered, you should add it to your default
or backend sections :)

Does that help?

Regards,
Aurelien

Re: [PATCH] FEATURE: Adding MPTCP with option to disable it and fall-back to TCP

[Matthieu Baerts]

Hi Willy,

Thank you for the feedback!

On 25/04/2024 00:12, Willy Tarreau wrote:
> Hi!
> 
> On Wed, Apr 24, 2024 at 05:45:03PM +0200, Nicolas CARPi wrote:
>> Hello,
>>
>> On 24 Apr, Dorian Craps wrote:
>>> This attached patch uses MPTCP by default instead of TCP on Linux. 
>> The backward compatibility of MPTCP is indeed a good point toward 
>> enabling it by default. Nonetheless, I feel your message should include 
>> a discussion on the security implications of this change.
>>
>> As you know, v0 had security issues. v1 addresses them, and we can 
>> likely consider that new attacks targeting this protocol will pop up as 
>> it becomes widespread.
>>
>> In fact, that's already the case:
>>
>> See: CVE-2024-26708: mptcp: really cope with fastopen race
>> or CVE-2024-26826: mptcp: fix data re-injection from stale subflow
>> or CVE-2024-26782 kernel: mptcp: fix double-free on socket dismantle
>>
>> The three CVEs above are all from April 2024.
>>
>> Given that MPTCP v1 is relatively new (2020), and that we do not have 
>> real assurances that it is at least as secure as plain TCP, I would 
>> humbly suggest inverting the logic, and making it an opt-in option.
>>
>> This way, a vulnerability impacting MPTCP would only impact users that 
>> enabled it, instead of 100% of HAProxy users. In a few years, making it 
>> the default could be reconsidered.
>>
>> Please note that I'm simply voicing my concern as a user, and the core 
>> dev team might have a very different view about these aspects.
>>
>>> It sounds good to have MPTCP enabled by default
>> Except when looking at it through the prism of the increased attack 
>> surface! ;)
>>
>>> IPPROTO_MPTCP is defined just in case old libC are being used and 
>>> don't have the ref.
>> Shouldn't it be defined with a value, as per 
>> https://www.mptcp.dev/faq.html#why-is-ipproto_mptcp-not-defined ?
>> (sorry if it's a dumb remark, I'm not a C dev)
> 
> Without going into all the details and comments above, I just want to
> say that I'll study this after 3.0 is released, since there's still a
> massive amount of stuff to adjust for the release and we're way past
> a feature freeze.

It makes sense, take your time!

> I *am* interested in the feature, which has been
> floating around for a few years already. However I tend to agree with
> Nicolas that, at least for the principle of least surprise, I'd rather
> not have this turned on by default. There might even be security
> implications that some are not willing to take yet, and forcing them
> to guess that they need to opt out of something is not great in general
> (it might change in the future though, once everyone turns it on by
> default, like we did for keep-alive, threads, and h2 for example).

It makes sense as well! I initially suggested to Dorian to first send a
patch where MPTCP is enabled by default because of the low impact (and
also after having seen an old comment on that topic [1]). But indeed,
doing that in steps sounds safer.

[1] https://github.com/haproxy/haproxy/issues/1028#issuecomment-754560146

> I'm also concerned by the change at the socket layer that will make all
> new sockets cause two syscalls on systems where this is not enabled,

I thought the listening sockets were created only once at startup and
having two syscalls would have been fine. I guess it should be easy to
remember the failure the first time, to avoid the extra syscalls for the
next listening sockets.

> and I'd really really prefer that we use the extended syntax for
> addresses that offers a lot of flexibility. We can definitely have
> "mptcp@address" and probably mptcp as a variant of tcp.

>From what I understood, Dorian is now looking at that. It is not clear
if he should create a new proto (struct protocol) or modifying it after
having called protocol_lookup() in str2sa_range().

Can MPTCP be used as a variant of "rhttp" as well?

> Regarding how
> we'd deal with the fallback, we'll see, but overall, thinking that> someone 
> would explicitly configure mptcp and not even get an error if
> it cannot bind is problematic to me, because among the most common
> causes of trouble are things that everyone ignores (module not loaded,
> missing secondary IP, firewall rules etc) that usually cause problems.
> Those we can discover at boot time should definitely be reported.

With the v1 Dorian suggested where MPTCP is enabled by default, a
warning is reported if it is not possible to create an MPTCP socket, and
a "plain" TCP one has been created instead.

If MPTCP is explicitly asked, I guess it would make sense to fail if it
is not possible to create an MPTCP 

Re: [PATCH] FEATURE: Adding MPTCP with option to disable it and fall-back to TCP

[Matthieu Baerts]

Hello Nicolas,

Thank you for the feedback!

On 24/04/2024 17:45, Nicolas CARPi wrote:
> Hello,
> 
> On 24 Apr, Dorian Craps wrote:
>> This attached patch uses MPTCP by default instead of TCP on Linux. 
> The backward compatibility of MPTCP is indeed a good point toward 
> enabling it by default. Nonetheless, I feel your message should include 
> a discussion on the security implications of this change.
> 
> As you know, v0 had security issues. v1 addresses them, and we can 
> likely consider that new attacks targeting this protocol will pop up as 
> it becomes widespread.

Indeed, any new features (or code in general) can bring security issues.
Same for the protocol. Here it looks like MPTCPv1 addressed all reported
issues [1].

> In fact, that's already the case:
> 
> See: CVE-2024-26708: mptcp: really cope with fastopen race
> or CVE-2024-26826: mptcp: fix data re-injection from stale subflow
> or CVE-2024-26782 kernel: mptcp: fix double-free on socket dismantle
> 
> The three CVEs above are all from April 2024.

:-)

There were no CVEs linked to MPTCP before Linux being a CNA earlier this
year [2]. Now any fixes could automatically get a CVE even if there is
no proven security issues [3]. For example, I don't think CVE-2024-26826
can cause security issues, and TCP also got some CVEs recently [4] :)

But anyway, here the CVEs are linked to the implementation in the Linux
kernel, not the protocol (MPTCPv1).

> Given that MPTCP v1 is relatively new (2020), and that we do not have 
> real assurances that it is at least as secure as plain TCP, I would 
> humbly suggest inverting the logic, and making it an opt-in option.
> 
> This way, a vulnerability impacting MPTCP would only impact users that 
> enabled it, instead of 100% of HAProxy users. In a few years, making it 
> the default could be reconsidered.

I understand that it is safer, first, to have an option to enable MPTCP
support, then enabling it by default later. But I admit that "In a few
years" sounds like "a very long time"! MPTCP has been introduced in
2020, 4 years ago, after more than two year of development. So it had
time to mature a bit in 6+ years :)
Still under, TCP is being used, it is not a full network implementation.

When MPTCP is enabled on the server side, it will only be used when
clients request it, without impacting "plain" TCP connections. So
enabling it will only impact connections where the client asked to use
MPTCP. I agree that it increases the attack surface, but like any new
features, no? :)

> Please note that I'm simply voicing my concern as a user, and the core 
> dev team might have a very different view about these aspects.
> 
>> It sounds good to have MPTCP enabled by default
> Except when looking at it through the prism of the increased attack 
> surface! ;)
> 
>> IPPROTO_MPTCP is defined just in case old libC are being used and 
>> don't have the ref.
> Shouldn't it be defined with a value, as per 
> https://www.mptcp.dev/faq.html#why-is-ipproto_mptcp-not-defined ?
> (sorry if it's a dumb remark, I'm not a C dev)

IPPROTO_MPTCP is a constant, it is 262. It is just that at build time,
some old libC might not have IPPROTO_MPTCP. To avoid compilation errors,
IPPROTO_MPTCP is defined if it was not before. MPTCP support is checked
at run time.


[1]
https://datatracker.ietf.org/doc/html/rfc8684#name-security-considerations
[2] http://www.kroah.com/log/blog/2024/02/13/linux-is-a-cna/
[3] https://docs.kernel.org/process/cve.html
[4] https://lore.kernel.org/linux-cve-announce/?q=s%3Atcp

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH 2/3] MINOR: Add `ha_generate_uuid_v7`

[Willy Tarreau]

On Thu, Apr 25, 2024 at 08:15:30PM +0200, Tim Düsterhus wrote:
> Hi
> 
> On 4/24/24 08:39, Willy Tarreau wrote:
> > Just thinking about all the shifts above, I think you could have
> > gone through less efforts by acting on 64-bit randoms (less shifts).
> > But the difference is probably not that much anyway.
> 
> I've used the existing implementation for UUIDv4 as the basis, that's why
> the code looks like it does.

Ah OK makes sense ;-)

Willy

Re: [PATCH 2/3] MINOR: Add `ha_generate_uuid_v7`

[Tim Düsterhus]

Hi

On 4/24/24 08:39, Willy Tarreau wrote:

Just thinking about all the shifts above, I think you could have
gone through less efforts by acting on 64-bit randoms (less shifts).
But the difference is probably not that much anyway.


I've used the existing implementation for UUIDv4 as the basis, that's 
why the code looks like it does.


Best regards
Tim Düsterhus

Re: [PATCH] MINOR: config: rhttp: Downgrade error on attach-srv name parsing

[Amaury Denoyelle]

Hi William !

Sorry for the delay. We have rediscussed this issue this morning and
here is my answer on your patch.

It is definitely legitimate to want to be able to use reverse HTTP
without SSL on the server line. However, the way that haproxy currently
uses idle connection is that at least the SNI parameter alone must be
set to match the name parameter of the corresponding attach-srv rule. If
this is not the case, the connection will never be reused.

But in fact, when rereading haproxy code this morning, we found that it
is valid to have SNI parameter set even if SSL is not active, and this
will produce the desired effect. We are definitely okay with merging an
adjusted version of your patch for the moment, see my remarks below.
However, using SNI on a server line without SSL is something tricky.
Thus we plan to add a new keyword to replace it when SSL is not used to
have the same effect. When this will be done, you should update your
configuration to use it.

On Fri, Apr 12, 2024 at 02:29:30PM +0100, William Manley wrote:
> An attach-srv config line usually looks like this:
> tcp-request session attach-srv be/srv name ssl_c_s_dn(CN)
> The name is a key that is used when looking up connections in the
> connection pool.  Without this patch you'd get an error if you passed
> anything other than "ssl_c_s_dn(CN)" as the name expression.  Now you can
> pass arbitrary expressions and it will just warn you if you aren't
> producing a configuration that is RFC compliant.
> I'm doing this as I want to use `fc_pp_unique_id` as the name.

I'm not 100% okay with your description here. The current code condition
does not check that "ssl_c_s_dn(CN)" is used as expression, but rather
that if name is defined for an attach-srv rule, the targetted server
must have both SSL and SNI activated. I think this paragraph should be
reworded.

> ---
>  src/tcp_act.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> diff --git a/src/tcp_act.c b/src/tcp_act.c
> index a88fab4af..4d2a56c67 100644
> --- a/src/tcp_act.c
> +++ b/src/tcp_act.c
> @@ -522,8 +522,7 @@ static int tcp_check_attach_srv(struct act_rule *rule, 
> struct proxy *px, char **
>  
>   if ((rule->arg.attach_srv.name && (!srv->use_ssl || !srv->sni_expr)) ||
>   (!rule->arg.attach_srv.name && srv->use_ssl && srv->sni_expr)) {
> - memprintf(err, "attach-srv rule: connection will never be used; 
> either specify name argument in conjunction with defined SSL SNI on targeted 
> server or none of these");
> - return 0;
> + ha_warning("attach-srv rule: connection may never be used; 
> usually name argument is defined SSL SNI on targeted server or none of 
> these");
>   }
>  
>   rule->arg.attach_srv.srv = srv;
> -- 
> 2.34.1
> 

I think an alternative patch may be more desirable here. We could update
the condition with the following statement without removing the fatal
error :

>   if ((rule->arg.attach_srv.name && !srv->sni_expr) ||
>   (!rule->arg.attach_srv.name && srv->sni_expr)) {

This reflects the current mandatory usage of reverse-http : if name is
used on attach-srv, sni keyword must be specified on the server line.

Let me know your thoughts. If you're okay, can you adjust your patch
please ? If not, do not hesitate to tell me if there is something you
disagreeing with.

Thanks,

-- 
Amaury Denoyelle

Re: [PATCH] FEATURE: Adding MPTCP with option to disable it and fall-back to TCP

[Willy Tarreau]

Hi!

On Wed, Apr 24, 2024 at 05:45:03PM +0200, Nicolas CARPi wrote:
> Hello,
> 
> On 24 Apr, Dorian Craps wrote:
> > This attached patch uses MPTCP by default instead of TCP on Linux. 
> The backward compatibility of MPTCP is indeed a good point toward 
> enabling it by default. Nonetheless, I feel your message should include 
> a discussion on the security implications of this change.
> 
> As you know, v0 had security issues. v1 addresses them, and we can 
> likely consider that new attacks targeting this protocol will pop up as 
> it becomes widespread.
> 
> In fact, that's already the case:
> 
> See: CVE-2024-26708: mptcp: really cope with fastopen race
> or CVE-2024-26826: mptcp: fix data re-injection from stale subflow
> or CVE-2024-26782 kernel: mptcp: fix double-free on socket dismantle
> 
> The three CVEs above are all from April 2024.
> 
> Given that MPTCP v1 is relatively new (2020), and that we do not have 
> real assurances that it is at least as secure as plain TCP, I would 
> humbly suggest inverting the logic, and making it an opt-in option.
> 
> This way, a vulnerability impacting MPTCP would only impact users that 
> enabled it, instead of 100% of HAProxy users. In a few years, making it 
> the default could be reconsidered.
> 
> Please note that I'm simply voicing my concern as a user, and the core 
> dev team might have a very different view about these aspects.
> 
> > It sounds good to have MPTCP enabled by default
> Except when looking at it through the prism of the increased attack 
> surface! ;)
> 
> > IPPROTO_MPTCP is defined just in case old libC are being used and 
> > don't have the ref.
> Shouldn't it be defined with a value, as per 
> https://www.mptcp.dev/faq.html#why-is-ipproto_mptcp-not-defined ?
> (sorry if it's a dumb remark, I'm not a C dev)

Without going into all the details and comments above, I just want to
say that I'll study this after 3.0 is released, since there's still a
massive amount of stuff to adjust for the release and we're way past
a feature freeze. I *am* interested in the feature, which has been
floating around for a few years already. However I tend to agree with
Nicolas that, at least for the principle of least surprise, I'd rather
not have this turned on by default. There might even be security
implications that some are not willing to take yet, and forcing them
to guess that they need to opt out of something is not great in general
(it might change in the future though, once everyone turns it on by
default, like we did for keep-alive, threads, and h2 for example).

I'm also concerned by the change at the socket layer that will make all
new sockets cause two syscalls on systems where this is not enabled,
and I'd really really prefer that we use the extended syntax for
addresses that offers a lot of flexibility. We can definitely have
"mptcp@address" and probably mptcp as a variant of tcp. Regarding how
we'd deal with the fallback, we'll see, but overall, thinking that
someone would explicitly configure mptcp and not even get an error if
it cannot bind is problematic to me, because among the most common
causes of trouble are things that everyone ignores (module not loaded,
missing secondary IP, firewall rules etc) that usually cause problems.
Those we can discover at boot time should definitely be reported. But
let's discuss this in early June instead (I mean, feel free to discuss
the points together, but I'm not going to invest more time on this at
this moment).

Thanks!
Willy

Re: [PATCH] FEATURE: Adding MPTCP with option to disable it and fall-back to TCP

[Nicolas CARPi]

Hello,

On 24 Apr, Dorian Craps wrote:
> This attached patch uses MPTCP by default instead of TCP on Linux. 
The backward compatibility of MPTCP is indeed a good point toward 
enabling it by default. Nonetheless, I feel your message should include 
a discussion on the security implications of this change.

As you know, v0 had security issues. v1 addresses them, and we can 
likely consider that new attacks targeting this protocol will pop up as 
it becomes widespread.

In fact, that's already the case:

See: CVE-2024-26708: mptcp: really cope with fastopen race
or CVE-2024-26826: mptcp: fix data re-injection from stale subflow
or CVE-2024-26782 kernel: mptcp: fix double-free on socket dismantle

The three CVEs above are all from April 2024.

Given that MPTCP v1 is relatively new (2020), and that we do not have 
real assurances that it is at least as secure as plain TCP, I would 
humbly suggest inverting the logic, and making it an opt-in option.

This way, a vulnerability impacting MPTCP would only impact users that 
enabled it, instead of 100% of HAProxy users. In a few years, making it 
the default could be reconsidered.

Please note that I'm simply voicing my concern as a user, and the core 
dev team might have a very different view about these aspects.

> It sounds good to have MPTCP enabled by default
Except when looking at it through the prism of the increased attack 
surface! ;)

> IPPROTO_MPTCP is defined just in case old libC are being used and 
> don't have the ref.
Shouldn't it be defined with a value, as per 
https://www.mptcp.dev/faq.html#why-is-ipproto_mptcp-not-defined ?
(sorry if it's a dumb remark, I'm not a C dev)

Best regards,
~Nicolas

Re: Fwd: [PATCH] MEDIUM: shctx naming shared memory context

[Willy Tarreau]

On Wed, Apr 24, 2024 at 09:53:04AM +0100, David CARLIER wrote:
> -- Forwarded message -
> From: David CARLIER 
> Date: Wed, 24 Apr 2024 at 07:56
> Subject: Re: [PATCH] MEDIUM: shctx naming shared memory context
> To: Willy Tarreau 
> 
> 
> Here a new version.

Works fine and applied, thank you. I'm seeing opportunities to use that
in other places like rings. It could require some tiny API changes however
to consistently pass the name. But that would be nice for ring buffers used
by traces, logs, startup-logs etc. Maybe we'll then decide to re-adjust the
displayed name to prepend a type. E.g "haproxy cache: my_cache_name" etc.
We're large with 80 chars since our identifiers are apparently limited to 32.

Cheers,
Willy

Re: [PATCH] MEDIUM: shctx naming shared memory context

[Willy Tarreau]

Hi David,

On Sat, Apr 20, 2024 at 07:33:16AM +0100, David CARLIER wrote:
> From d49d9d5966caead320f33f789578cb69f2aa3787 Mon Sep 17 00:00:00 2001
> From: David Carlier 
> Date: Sat, 20 Apr 2024 07:18:48 +0100
> Subject: [PATCH] MEDIUM: shctx: Naming shared memory context
> 
> From Linux 5.17, anonymous regions can be name via prctl/PR_SET_VMA
> so caches can be identified when looking at HAProxy process memory
> mapping.

That's pretty cool, I wasn't aware of this feature, that's really
interesting!

However I disagree with this point:

> +#if defined(USE_PRCTL) && defined(PR_SET_VMA)
> + {
> + /**
> +  * From Linux 5.17 (and if the `CONFIG_ANON_VMA_NAME` kernel 
> config is set)`,
> +  * anonymous regions can be named.
> +  *
> +  * The naming can take up to 79 characters, accepting valid 
> ASCII values
> +  * except [, ], \, $ and '.
> +  * As a result, when looking for /proc//maps, we can see 
> the anonymous range
> +  * as follow :
> +  * `7364c4fff000-73650800 rw-s  00:01 3540  
> [anon_shmem:HAProxy globalCache]`
> +  */
> + int rn;
> + char fullname[80];
> +
> + rn = snprintf(fullname, sizeof(fullname), "HAProxy %s", name);
> + if (rn < 0 || prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, 
> (uintptr_t)shctx,
> +totalsize, (uintptr_t)fullname) != 0) {
> + munmap(shctx, totalsize);
> + shctx = NULL;
> + ret = SHCTX_E_ALLOC_CACHE;
> + goto err;
> + }
> + }
> +#endif

You're making the mapping fail on any kernel that does not support the
feature (hence apparently anything < 5.17 according to your description).
IMHO we should just silently fall back to the default behavior, i.e. we
couldn't assign the name, fine, we continue to run without a name, thus
something like this:

rn = snprintf(fullname, sizeof(fullname), "HAProxy %s", name);
if (rn >= 0)
prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, (uintptr_t)shctx, 
totalsize, (uintptr_t)fullname);

Could you please adjust it and verify that it's still OK for you ?
Likewise I can check it on a 5.15 here.

Thank you!
Willy

Re: [PATCH 2/3] MINOR: Add `ha_generate_uuid_v7`

[Willy Tarreau]

Hi Tim!

On Fri, Apr 19, 2024 at 09:01:26PM +0200, Tim Duesterhus wrote:
> +/* Generates a draft-ietf-uuidrev-rfc4122bis-14 version 7 UUID into chunk
> + *  which must be at least 37 bytes large.
> + */
> +void ha_generate_uuid_v7(struct buffer *output)
> +{
> + uint32_t rnd[3];
> + uint64_t last;
> + uint64_t time;
> +
> + time = (date.tv_sec * 1000) + (date.tv_usec / 1000);
> + last = ha_random64();
> + rnd[0] = last;
> + rnd[1] = last >> 32;
> +
> + last = ha_random64();
> + rnd[2] = last;
> +
> + chunk_printf(output, "%8.8x-%4.4x-%4.4x-%4.4x-%12.12llx",
> +  (uint)(time >> 16u),
> +  (uint)(time & 0x),
> +  ((rnd[0] >> 16u) & 0xFFF) | 0x7000,  // highest 4 bits 
> indicate the uuid version
> +  (rnd[1] & 0x3FFF) | 0x8000,  // the highest 2 bits 
> indicate the UUID variant (10),
> +  (long long)((rnd[1] >> 14u) | ((uint64_t) rnd[2] << 18u)) 
> & 0xull);
> +}

Just thinking about all the shifts above, I think you could have
gone through less efforts by acting on 64-bit randoms (less shifts).
But the difference is probably not that much anyway.

In any case, that looks good and I've merged it now. Many thanks!
Willy

Re: [PATCH 0/3] Add support for UUIDv7

[Willy Tarreau]

Hi Tim,

On Tue, Apr 23, 2024 at 09:03:32PM +0200, Tim Düsterhus wrote:
> Hi
> 
> On 4/19/24 21:07, Willy Tarreau wrote:
> > it! I'll have a look on Monday, I'm really done for this week, need to
> 
> Monday is gone. So here's a friendly reminder :-)

Yeah and I'm sorry, my week started with two day-long meetings, I met my
keyboard first around 6pm :-/  I'm doing my best for tomorrow morning, I
promis I'm not forgetting you (nor David who also sent something).

Thanks,
Willy

Re: [PATCH 0/3] Add support for UUIDv7

[Tim Düsterhus]

Hi

On 4/19/24 21:07, Willy Tarreau wrote:

it! I'll have a look on Monday, I'm really done for this week, need to


Monday is gone. So here's a friendly reminder :-)

Best regards
Tim Düsterhus

Re: Changes in HAProxy 3.0's Makefile and build options

[Илья Шипицин]

I'll postpone for a while.
I thought that value of "2" is the same as "1", I'll try to find better doc.

seems that I didn''t specify "march" and that might be the cause.

сб, 20 апр. 2024 г. в 15:21, Willy Tarreau :

> On Sat, Apr 20, 2024 at 03:11:19PM +0200,  ??? wrote:
> > ??, 20 ???. 2024 ?. ? 15:07, Willy Tarreau :
> >
> > > On Sat, Apr 20, 2024 at 02:49:38PM +0200,  ??? wrote:
> > > > ??, 11 ???. 2024 ?. ? 21:05, Willy Tarreau :
> > > >
> > > > > Hi Ilya,
> > > > >
> > > > > On Thu, Apr 11, 2024 at 08:27:39PM +0200,  ??? wrote:
> > > > > > do you know maybe how this was supposed to work ?
> > > > > > haproxy/Makefile at master · haproxy/haproxy (github.com)
> > > > > > 
> > > > >
> > > > > That's this:
> > > > >
> > > > >   ifneq ($(shell $(CC) $(CFLAGS) -dM -E -xc -  2>/dev/null |
> > > > > grep -c 'LOCK_FREE.*1'),0)
> > > > > USE_LIBATOMIC   = implicit
> > > > >   endif
> > > > >
> > > > > It calls the compiler with the known flags and checks if for this
> arch,
> > > > > it's configured to require libatomic.
> > > > >
> > > >
> > > > macros has changed from 1 to 2:
> > > >
> > > > ilia@fedora:~/Downloads$ cc -dM -E -xc - /dev/null  |
> grep
> > > LOCK
> > > > #define __GCC_ATOMIC_CHAR_LOCK_FREE 2
> > > > #define __GCC_ATOMIC_CHAR32_T_LOCK_FREE 2
> > > > #define __GCC_ATOMIC_BOOL_LOCK_FREE 2
> > > > #define __GCC_ATOMIC_POINTER_LOCK_FREE 2
> > > > #define __GCC_ATOMIC_INT_LOCK_FREE 2
> > > > #define __GCC_ATOMIC_WCHAR_T_LOCK_FREE 2
> > > > #define __GCC_ATOMIC_LONG_LOCK_FREE 2
> > > > #define __GCC_ATOMIC_CHAR16_T_LOCK_FREE 2
> > > > #define __GCC_ATOMIC_LLONG_LOCK_FREE 2
> > > > #define __GCC_ATOMIC_SHORT_LOCK_FREE 2
> > >
> > > This means it's always lock-free, implemented natively thus doesn't
> > > require libatomic. Value 1 means "sometimes lock-free" and implemented
> > > as a function provided by libatomic.
> > >
> > > Did the problem appear when I changed the flags in the makefile ? Maybe
> > > I accidently lost one and it's falling back to a subset of the target
> > > arch ?
> > >
> >
> >
> > the problem appears only with QuicTLS manually built with "-m32" flag. it
> > does not appear with "-m32" if built and linked against system OpenSS:L
> >
> > but after I modify condition (the same as previously enforcing libatomic
> in
> > ADDLIB), it builds fine.
>
> OK thanks for that, but was it already present before my changes in the
> makefile ? Could you check that the -m32 flag is properly passed to this
> test ?
>
> On 32-bit ix86, there are different cases that require libatomic:
>
>   $ gcc -march=i686 -m32 -dM -E -xc - < /dev/null |grep -c LOCK.*1
>   0
>   $ gcc -march=i586 -m32 -dM -E -xc - < /dev/null |grep -c LOCK.*1
>   0
>   $ gcc -march=i486 -m32 -dM -E -xc - < /dev/null |grep -c LOCK.*1
>   1
>   $ gcc -march=i386 -m32 -dM -E -xc - < /dev/null |grep -c LOCK.*1
>   10
>
> Only i386 and i486 require it. That makes me think, maybe it's quictls
> that was built against it and adds a dependency to it. You can check
> it using objdump -p|grep NEEDED.
>
> If so that would make sense to just manually add it (or any other
> required dependencies) in ADDLIB since they're here just to satisfy
> external dependencies.
>
> Willy
>

Re: Changes in HAProxy 3.0's Makefile and build options

[Willy Tarreau]

On Sat, Apr 20, 2024 at 03:11:19PM +0200,  ??? wrote:
> ??, 20 ???. 2024 ?. ? 15:07, Willy Tarreau :
> 
> > On Sat, Apr 20, 2024 at 02:49:38PM +0200,  ??? wrote:
> > > ??, 11 ???. 2024 ?. ? 21:05, Willy Tarreau :
> > >
> > > > Hi Ilya,
> > > >
> > > > On Thu, Apr 11, 2024 at 08:27:39PM +0200,  ??? wrote:
> > > > > do you know maybe how this was supposed to work ?
> > > > > haproxy/Makefile at master · haproxy/haproxy (github.com)
> > > > > 
> > > >
> > > > That's this:
> > > >
> > > >   ifneq ($(shell $(CC) $(CFLAGS) -dM -E -xc - /dev/null |
> > > > grep -c 'LOCK_FREE.*1'),0)
> > > > USE_LIBATOMIC   = implicit
> > > >   endif
> > > >
> > > > It calls the compiler with the known flags and checks if for this arch,
> > > > it's configured to require libatomic.
> > > >
> > >
> > > macros has changed from 1 to 2:
> > >
> > > ilia@fedora:~/Downloads$ cc -dM -E -xc - /dev/null  | grep
> > LOCK
> > > #define __GCC_ATOMIC_CHAR_LOCK_FREE 2
> > > #define __GCC_ATOMIC_CHAR32_T_LOCK_FREE 2
> > > #define __GCC_ATOMIC_BOOL_LOCK_FREE 2
> > > #define __GCC_ATOMIC_POINTER_LOCK_FREE 2
> > > #define __GCC_ATOMIC_INT_LOCK_FREE 2
> > > #define __GCC_ATOMIC_WCHAR_T_LOCK_FREE 2
> > > #define __GCC_ATOMIC_LONG_LOCK_FREE 2
> > > #define __GCC_ATOMIC_CHAR16_T_LOCK_FREE 2
> > > #define __GCC_ATOMIC_LLONG_LOCK_FREE 2
> > > #define __GCC_ATOMIC_SHORT_LOCK_FREE 2
> >
> > This means it's always lock-free, implemented natively thus doesn't
> > require libatomic. Value 1 means "sometimes lock-free" and implemented
> > as a function provided by libatomic.
> >
> > Did the problem appear when I changed the flags in the makefile ? Maybe
> > I accidently lost one and it's falling back to a subset of the target
> > arch ?
> >
> 
> 
> the problem appears only with QuicTLS manually built with "-m32" flag. it
> does not appear with "-m32" if built and linked against system OpenSS:L
> 
> but after I modify condition (the same as previously enforcing libatomic in
> ADDLIB), it builds fine.

OK thanks for that, but was it already present before my changes in the
makefile ? Could you check that the -m32 flag is properly passed to this
test ?

On 32-bit ix86, there are different cases that require libatomic:

  $ gcc -march=i686 -m32 -dM -E -xc - < /dev/null |grep -c LOCK.*1
  0
  $ gcc -march=i586 -m32 -dM -E -xc - < /dev/null |grep -c LOCK.*1
  0
  $ gcc -march=i486 -m32 -dM -E -xc - < /dev/null |grep -c LOCK.*1
  1
  $ gcc -march=i386 -m32 -dM -E -xc - < /dev/null |grep -c LOCK.*1
  10

Only i386 and i486 require it. That makes me think, maybe it's quictls
that was built against it and adds a dependency to it. You can check
it using objdump -p|grep NEEDED.

If so that would make sense to just manually add it (or any other
required dependencies) in ADDLIB since they're here just to satisfy
external dependencies.

Willy

Re: Changes in HAProxy 3.0's Makefile and build options

[Илья Шипицин]

сб, 20 апр. 2024 г. в 15:07, Willy Tarreau :

> On Sat, Apr 20, 2024 at 02:49:38PM +0200,  ??? wrote:
> > ??, 11 ???. 2024 ?. ? 21:05, Willy Tarreau :
> >
> > > Hi Ilya,
> > >
> > > On Thu, Apr 11, 2024 at 08:27:39PM +0200,  ??? wrote:
> > > > do you know maybe how this was supposed to work ?
> > > > haproxy/Makefile at master · haproxy/haproxy (github.com)
> > > > 
> > >
> > > That's this:
> > >
> > >   ifneq ($(shell $(CC) $(CFLAGS) -dM -E -xc - /dev/null |
> > > grep -c 'LOCK_FREE.*1'),0)
> > > USE_LIBATOMIC   = implicit
> > >   endif
> > >
> > > It calls the compiler with the known flags and checks if for this arch,
> > > it's configured to require libatomic.
> > >
> >
> > macros has changed from 1 to 2:
> >
> > ilia@fedora:~/Downloads$ cc -dM -E -xc - /dev/null  | grep
> LOCK
> > #define __GCC_ATOMIC_CHAR_LOCK_FREE 2
> > #define __GCC_ATOMIC_CHAR32_T_LOCK_FREE 2
> > #define __GCC_ATOMIC_BOOL_LOCK_FREE 2
> > #define __GCC_ATOMIC_POINTER_LOCK_FREE 2
> > #define __GCC_ATOMIC_INT_LOCK_FREE 2
> > #define __GCC_ATOMIC_WCHAR_T_LOCK_FREE 2
> > #define __GCC_ATOMIC_LONG_LOCK_FREE 2
> > #define __GCC_ATOMIC_CHAR16_T_LOCK_FREE 2
> > #define __GCC_ATOMIC_LLONG_LOCK_FREE 2
> > #define __GCC_ATOMIC_SHORT_LOCK_FREE 2
>
> This means it's always lock-free, implemented natively thus doesn't
> require libatomic. Value 1 means "sometimes lock-free" and implemented
> as a function provided by libatomic.
>
> Did the problem appear when I changed the flags in the makefile ? Maybe
> I accidently lost one and it's falling back to a subset of the target
> arch ?
>


the problem appears only with QuicTLS manually built with "-m32" flag. it
does not appear with "-m32" if built and linked against system OpenSS:L

but after I modify condition (the same as previously enforcing libatomic in
ADDLIB), it builds fine.


>
> > the following patch works, but I'll play a bit more 
> >
> > diff --git a/Makefile b/Makefile
> > index 4bd263498..370ac7ed0 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -493,7 +493,7 @@ $(set_target_defaults)
> >  # linking with it by default as it's not always available nor deployed
> >  # (especially on archs which do not need it).
> >  ifneq ($(USE_THREAD:0=),)
> > -  ifneq ($(shell $(CC) $(OPT_CFLAGS) $(ARCH_FLAGS) $(CPU_CFLAGS)
> > $(STD_CFLAGS) $(WARN_CFLAGS) $(NOWARN_CFLAGS) $(ERROR_CFLAGS) $(CFLAGS)
> -dM
> > -E -xc - /dev/null | grep -c 'LOCK_FREE.*1'),0)
> > +  ifneq ($(shell $(CC) $(OPT_CFLAGS) $(ARCH_FLAGS) $(CPU_CFLAGS)
> > $(STD_CFLAGS) $(WARN_CFLAGS) $(NOWARN_CFLAGS) $(ERROR_CFLAGS) $(CFLAGS)
> -dM
> > -E -xc - /dev/null | grep -c 'LOCK_FREE.*[12]'),0)
> >  USE_LIBATOMIC   = implicit
> >endif
> >  endif
>
> It would impose libatomic for everyone, and not everyone has it. For
> example this would break clang on freebsd from what I'm seeing. That's
> not logic, there must be another reason.
>
> Willy
>

Re: Changes in HAProxy 3.0's Makefile and build options

[Willy Tarreau]

On Sat, Apr 20, 2024 at 02:49:38PM +0200,  ??? wrote:
> ??, 11 ???. 2024 ?. ? 21:05, Willy Tarreau :
> 
> > Hi Ilya,
> >
> > On Thu, Apr 11, 2024 at 08:27:39PM +0200,  ??? wrote:
> > > do you know maybe how this was supposed to work ?
> > > haproxy/Makefile at master · haproxy/haproxy (github.com)
> > > 
> >
> > That's this:
> >
> >   ifneq ($(shell $(CC) $(CFLAGS) -dM -E -xc - /dev/null |
> > grep -c 'LOCK_FREE.*1'),0)
> > USE_LIBATOMIC   = implicit
> >   endif
> >
> > It calls the compiler with the known flags and checks if for this arch,
> > it's configured to require libatomic.
> >
> 
> macros has changed from 1 to 2:
> 
> ilia@fedora:~/Downloads$ cc -dM -E -xc - /dev/null  | grep LOCK
> #define __GCC_ATOMIC_CHAR_LOCK_FREE 2
> #define __GCC_ATOMIC_CHAR32_T_LOCK_FREE 2
> #define __GCC_ATOMIC_BOOL_LOCK_FREE 2
> #define __GCC_ATOMIC_POINTER_LOCK_FREE 2
> #define __GCC_ATOMIC_INT_LOCK_FREE 2
> #define __GCC_ATOMIC_WCHAR_T_LOCK_FREE 2
> #define __GCC_ATOMIC_LONG_LOCK_FREE 2
> #define __GCC_ATOMIC_CHAR16_T_LOCK_FREE 2
> #define __GCC_ATOMIC_LLONG_LOCK_FREE 2
> #define __GCC_ATOMIC_SHORT_LOCK_FREE 2

This means it's always lock-free, implemented natively thus doesn't
require libatomic. Value 1 means "sometimes lock-free" and implemented
as a function provided by libatomic.

Did the problem appear when I changed the flags in the makefile ? Maybe
I accidently lost one and it's falling back to a subset of the target
arch ?

> the following patch works, but I'll play a bit more 
> 
> diff --git a/Makefile b/Makefile
> index 4bd263498..370ac7ed0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -493,7 +493,7 @@ $(set_target_defaults)
>  # linking with it by default as it's not always available nor deployed
>  # (especially on archs which do not need it).
>  ifneq ($(USE_THREAD:0=),)
> -  ifneq ($(shell $(CC) $(OPT_CFLAGS) $(ARCH_FLAGS) $(CPU_CFLAGS)
> $(STD_CFLAGS) $(WARN_CFLAGS) $(NOWARN_CFLAGS) $(ERROR_CFLAGS) $(CFLAGS) -dM
> -E -xc - /dev/null | grep -c 'LOCK_FREE.*1'),0)
> +  ifneq ($(shell $(CC) $(OPT_CFLAGS) $(ARCH_FLAGS) $(CPU_CFLAGS)
> $(STD_CFLAGS) $(WARN_CFLAGS) $(NOWARN_CFLAGS) $(ERROR_CFLAGS) $(CFLAGS) -dM
> -E -xc - /dev/null | grep -c 'LOCK_FREE.*[12]'),0)
>  USE_LIBATOMIC   = implicit
>endif
>  endif

It would impose libatomic for everyone, and not everyone has it. For
example this would break clang on freebsd from what I'm seeing. That's
not logic, there must be another reason.

Willy

Re: Changes in HAProxy 3.0's Makefile and build options

[Илья Шипицин]

чт, 11 апр. 2024 г. в 21:05, Willy Tarreau :

> Hi Ilya,
>
> On Thu, Apr 11, 2024 at 08:27:39PM +0200,  ??? wrote:
> > do you know maybe how this was supposed to work ?
> > haproxy/Makefile at master · haproxy/haproxy (github.com)
> > 
>
> That's this:
>
>   ifneq ($(shell $(CC) $(CFLAGS) -dM -E -xc - /dev/null |
> grep -c 'LOCK_FREE.*1'),0)
> USE_LIBATOMIC   = implicit
>   endif
>
> It calls the compiler with the known flags and checks if for this arch,
> it's configured to require libatomic.
>

macros has changed from 1 to 2:

ilia@fedora:~/Downloads$ cc -dM -E -xc - /dev/null  | grep LOCK
#define __GCC_ATOMIC_CHAR_LOCK_FREE 2
#define __GCC_ATOMIC_CHAR32_T_LOCK_FREE 2
#define __GCC_ATOMIC_BOOL_LOCK_FREE 2
#define __GCC_ATOMIC_POINTER_LOCK_FREE 2
#define __GCC_ATOMIC_INT_LOCK_FREE 2
#define __GCC_ATOMIC_WCHAR_T_LOCK_FREE 2
#define __GCC_ATOMIC_LONG_LOCK_FREE 2
#define __GCC_ATOMIC_CHAR16_T_LOCK_FREE 2
#define __GCC_ATOMIC_LLONG_LOCK_FREE 2
#define __GCC_ATOMIC_SHORT_LOCK_FREE 2

the following patch works, but I'll play a bit more 

diff --git a/Makefile b/Makefile
index 4bd263498..370ac7ed0 100644
--- a/Makefile
+++ b/Makefile
@@ -493,7 +493,7 @@ $(set_target_defaults)
 # linking with it by default as it's not always available nor deployed
 # (especially on archs which do not need it).
 ifneq ($(USE_THREAD:0=),)
-  ifneq ($(shell $(CC) $(OPT_CFLAGS) $(ARCH_FLAGS) $(CPU_CFLAGS)
$(STD_CFLAGS) $(WARN_CFLAGS) $(NOWARN_CFLAGS) $(ERROR_CFLAGS) $(CFLAGS) -dM
-E -xc - /dev/null | grep -c 'LOCK_FREE.*1'),0)
+  ifneq ($(shell $(CC) $(OPT_CFLAGS) $(ARCH_FLAGS) $(CPU_CFLAGS)
$(STD_CFLAGS) $(WARN_CFLAGS) $(NOWARN_CFLAGS) $(ERROR_CFLAGS) $(CFLAGS) -dM
-E -xc - /dev/null | grep -c 'LOCK_FREE.*[12]'),0)
 USE_LIBATOMIC   = implicit
   endif
 endif



>
> > I had to enable atomic explicitly despite it was supposed to be detected
> on
> > the fly (I haven't had a deeper look yet)
> >
> > haproxy/.github/workflows/fedora-rawhide.yml at master · haproxy/haproxy
> > <
> https://github.com/haproxy/haproxy/blob/master/.github/workflows/fedora-rawhide.yml#L17-L18
> >
>
> I honestly don't know why it's not working, it could be useful to achive
> the output of this command, either by calling it directly or for example
> by inserting "tee /tmp/log |" before grep. If you have a copy of the linker
> erorr you got without the lib, maybe it will reveal something. But honestly
> this is a perfect example of the reasons why I prefer generic makefiles to
> automatic detection: you see that you need libatomic, you add it, done. No
> need to patch a configure to mask an error due to an unhandled special case
> etc. The test above was mostly meant to ease the build for the most common
> cases (and for me till now it has always worked, on various systems and
> hardware), but I wouldn't mind too much.
>
> In fact you could simplify your build script by just passing
> USE_LIBATOMIC=1 to enable it.
>
> BTW, is there a reason for "make -j3" in the build script ? Limiting
> oneself
> to 3 build processes when modern machines rarely have less than 8 cores is
> a bit of a waste of time, especially if every other package does the same
> in the distro! I'd just do "make -j$(nproc)" as usual there.
>
> Cheers,
> Willy
>

Re: Update for https://github.com/haproxy/wiki/wiki/SPOE:-Stream-Processing-Offloading-Engine

[William Lallemand]

On Mon, Apr 15, 2024 at 10:18:19AM +0200, Aleksandar Lazic wrote:
> Hi.
> 
> The "https://github.com/criteo/haproxy-spoe-go; is archived since Nov 7,
> 2023 and there is a fork from that repo https://github.com/go-spop/spoe
> Can we add this info to the wiki page?
> 
> There is also a rust implementation
> https://github.com/vkill/haproxy-spoa-example which could be added.
> 
> If it's possible then would I add this by my self.
> 

Thanks Aleks, I add them both on the page, and set criteo's one as
unmaintained.


-- 
William Lallemand

Re: [PATCH 0/3] Add support for UUIDv7

[Willy Tarreau]

Hi Tim!

On Fri, Apr 19, 2024 at 09:01:24PM +0200, Tim Duesterhus wrote:
> Willy,
> 
> as requested in the thread "[ANNOUNCE] haproxy-3.0-dev7":
> 
> > Regarding UUIDs, though, I've recently come across UUIDv7 which I found
> > particularly interesting, and that I think would be nice to implement
> > in the uuid() sample fetch function before 3.0 is released.
> 
> No reg-tests added, as those doesn't allow meaningfully testing that the
> UUIDv7 is actually a UUIDv7. I have manually checked the output against
> https://uuid7.com/.

Oh, many thanks for this, it was still on my todo list and I sense that
the review of your patches will take me less effort than implementing
it! I'll have a look on Monday, I'm really done for this week, need to
have some sleep.

Thanks and have a nice week-end!
Willy

Re: [PATCH 0/1] CI: switch to more recent macos version(s)

[Willy Tarreau]

On Fri, Apr 19, 2024 at 07:16:44AM +0200, Ilya Shipitsin wrote:
> let's modernize macos CI build matrix since macos-14 is available

Merged, thank you Ilya!
willy

Re: [PATCH 1/2] CI: reduce ASAN log redirection umbrella size

[Илья Шипицин]

on my experiments, asan log was grouped under "show vtest results".
on provided branch indeed there are no grouping.

I'll play a bit, maybe we'll end with dropping that log redirection

ср, 17 апр. 2024 г. в 21:17, William Lallemand :

> On Sun, Apr 14, 2024 at 09:23:51AM +0200, Ilya Shipitsin wrote:
> > previously ASAN_OPTIONS=log_path=asan.log was intended for VTest
> > execution only, it should not affect "haproxy -vv" and hsproxy
> > config smoke testing
> > ---
> >  .github/workflows/vtest.yml | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/.github/workflows/vtest.yml b/.github/workflows/vtest.yml
> > index 9d0bf48b0..5ee8a7a64 100644
> > --- a/.github/workflows/vtest.yml
> > +++ b/.github/workflows/vtest.yml
> > @@ -42,8 +42,6 @@ jobs:
> ># Configure a short TMPDIR to prevent failures due to long unix
> socket
> ># paths.
> >TMPDIR: /tmp
> > -  # Force ASAN output into asan.log to make the output more
> readable.
> > -  ASAN_OPTIONS: log_path=asan.log
> >OT_CPP_VERSION: 1.6.0
> >  steps:
> >  - uses: actions/checkout@v4
> > @@ -143,6 +141,9 @@ jobs:
> >run: echo "::add-matcher::.github/vtest.json"
> >  - name: Run VTest for HAProxy ${{
> steps.show-version.outputs.version }}
> >id: vtest
> > +  env:
> > +# Force ASAN output into asan.log to make the output more
> readable.
> > +ASAN_OPTIONS: log_path=asan.log
> >run: |
> >  # This is required for macOS which does not actually allow to
> increase
> >  # the '-n' soft limit to the hard limit, thus failing to run.
>
>
> Ilya,
>
> I still don't get how ASAN is working with the CI. Each time I have an
> ASAN issue I can't get a trace out of github.
>
> For example, there was an issue with ASAN in this commit:
>
> https://github.com/haproxy/haproxy/commit/bdee8ace814139771efa90cc200c67e7d9b72751
>
> I couldn't get a trace in the CI:
> https://github.com/haproxy/haproxy/actions/runs/8724600484/job/23936238899
>
> But I had no problem when testing it from my computer, I'm just doing a
> ` make reg-tests reg-tests/ssl/crt_store.vtc -- --debug` and have the
> ASAN output directly.
>
> Do you think we could achieve the same thing with github actions? I
> never saw an output from this asan.log file in the CI.
>
> --
> William Lallemand
>

Re: [PATCH 1/2] CI: reduce ASAN log redirection umbrella size

[William Lallemand]

On Sun, Apr 14, 2024 at 09:23:51AM +0200, Ilya Shipitsin wrote:
> previously ASAN_OPTIONS=log_path=asan.log was intended for VTest
> execution only, it should not affect "haproxy -vv" and hsproxy
> config smoke testing
> ---
>  .github/workflows/vtest.yml | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/.github/workflows/vtest.yml b/.github/workflows/vtest.yml
> index 9d0bf48b0..5ee8a7a64 100644
> --- a/.github/workflows/vtest.yml
> +++ b/.github/workflows/vtest.yml
> @@ -42,8 +42,6 @@ jobs:
># Configure a short TMPDIR to prevent failures due to long unix socket
># paths.
>TMPDIR: /tmp
> -  # Force ASAN output into asan.log to make the output more readable.
> -  ASAN_OPTIONS: log_path=asan.log
>OT_CPP_VERSION: 1.6.0
>  steps:
>  - uses: actions/checkout@v4
> @@ -143,6 +141,9 @@ jobs:
>run: echo "::add-matcher::.github/vtest.json"
>  - name: Run VTest for HAProxy ${{ steps.show-version.outputs.version }}
>id: vtest
> +  env:
> +# Force ASAN output into asan.log to make the output more readable.
> +ASAN_OPTIONS: log_path=asan.log
>run: |
>  # This is required for macOS which does not actually allow to 
> increase
>  # the '-n' soft limit to the hard limit, thus failing to run.


Ilya,

I still don't get how ASAN is working with the CI. Each time I have an
ASAN issue I can't get a trace out of github.

For example, there was an issue with ASAN in this commit:
https://github.com/haproxy/haproxy/commit/bdee8ace814139771efa90cc200c67e7d9b72751

I couldn't get a trace in the CI: 
https://github.com/haproxy/haproxy/actions/runs/8724600484/job/23936238899

But I had no problem when testing it from my computer, I'm just doing a
` make reg-tests reg-tests/ssl/crt_store.vtc -- --debug` and have the
ASAN output directly.

Do you think we could achieve the same thing with github actions? I
never saw an output from this asan.log file in the CI.

-- 
William Lallemand

Re: [PATCH 0/2] CI cleanup, spell fixes

[Willy Tarreau]

On Sun, Apr 14, 2024 at 09:23:50AM +0200, Ilya Shipitsin wrote:
> the main part is reducing ASAN_OPTIONS scope, it was supposed
> only to capture output of vtests, accidently it covered "config smoke tests" 
> as well
(...)
Both merged, thank you Ilya!
willy

Re: [PATCH] MINOR: cli: add option to modify close-spread-time

[Willy Tarreau]

Hi Abhijeet,

On Mon, Apr 15, 2024 at 09:48:25PM -0700, Abhijeet Rastogi wrote:
> Hi Willy,
> 
> Thank you for your patience with my questions.

You're welcome!

> > It happens that the global struct is only changed during startup
> 
> I used cli_parse_set_maxconn_global as a reference for my patch and my
> understanding is, it does have a race as I do not see
> thread_isolate().
> 
> https://github.com/haproxy/haproxy/blob/fa5c4cc6ced5d8cac2132593ed6b10cf3a8a4660/src/cli.c#L1762

Ah I see. I didn't remember we could change it at run time and it
predates threads. In practice there's no race since it writes only
a single value. It should theoretically use an atomic write here to
do things more cleanly but in reality all the platforms we support
are 32-bit and none will make non-atomic writes to an aligned 32-bit 
location.

In your case the difference is that you need to set both a value and
a flag and need to make them appear consistently to observers, which
is why you'd need this isolation.

> >That's easier than it seems if you set an ormask, andnotmask and the new 
> >value from a local variable.
> 
> Correct, however, I wasn't sure if it's okay to read the mask first,
> then get out of the critical section to do other stuff. (in case it's
> modified by other threads)

What I meant was:
  - parse the cmd line to local variables (value, ormask, andnotmask)
  - thread_isolate()
  - apply these local variables to global ones
  - thread_release()

> Also, it felt natural to minimize the use of thread_isolate(), hence,
> I went with a solution where we only use that once per execution. I
> will fix this if it doesn't make sense.

I'd also want to see only one :-)  But the way you've done it is fine,
a single one is executed for each "if" branch and the code remains
clean enough.

> >it would be better to call this "set global close-spread-time"
> 
> Good point, I've done it in the latest iteration.

Thanks. However you should not reindent the whole table like this in the
same patch, it makes the review way more complicated, and even later if
for any reason we need to recheck that patch, this part is horrendous.
The right approach when you feel like you need to reindent a table due
to some longer keywords, function names or so, is to make a preliminary
"cleanup" patch that only does the reindent and promises in the commit
message that nothing else was touched, and do you feature patch after
that one. This way the first one doesn't deserve any analysis and the
second one is clean.

I'm seeing that an entry is missing for "doc/management.txt" regarding
this new command, please write a few lines about it. Otherwise it's
generally OK for me.

Thank you!
Willy

Re: [PATCH] MINOR: cli: add option to modify close-spread-time

[Abhijeet Rastogi]

Hi Willy,

Thank you for your patience with my questions.

> It happens that the global struct is only changed during startup

I used cli_parse_set_maxconn_global as a reference for my patch and my
understanding is, it does have a race as I do not see
thread_isolate().

https://github.com/haproxy/haproxy/blob/fa5c4cc6ced5d8cac2132593ed6b10cf3a8a4660/src/cli.c#L1762

>That's easier than it seems if you set an ormask, andnotmask and the new value 
>from a local variable.

Correct, however, I wasn't sure if it's okay to read the mask first,
then get out of the critical section to do other stuff. (in case it's
modified by other threads)
Also, it felt natural to minimize the use of thread_isolate(), hence,
I went with a solution where we only use that once per execution. I
will fix this if it doesn't make sense.

>it would be better to call this "set global close-spread-time"

Good point, I've done it in the latest iteration.

On Sun, Apr 14, 2024 at 11:56 PM Willy Tarreau  wrote:
>
> Hi Abhijeet,
>
> On Mon, Apr 08, 2024 at 08:11:28PM -0700, Abhijeet Rastogi wrote:
> > Hi HAproxy community,
> >
> > Let's assume that HAproxy starts with non-zero values for close-spread-time
> > and hard-stop-after, and soft-stop is used to initiate the shutdown during
> > deployments.
> > There are times where we need to modify these values, eg, in case the
> > operator needs to restart HAproxy faster, but still use the soft-stop
> > workflow.
> >
> > So, it will be nice to have the ability to modify these values via CLI.
> >
> > RFC:-
> > - Does this seem like a fair use-case?
> > - If this is good, I can also work on the patch for hard-stop-after.
>
> Yes, I think that both totally make sense from a production perspective.
> I hadn't thought about that before, but it's obvious that when you've
> deployed with a wrong setting or are observing a bad behavior, you'd
> really like the old process to quit much faster.
>
> > Patch questions:-
> > - I've added reg-tests under a new folder "cli" because I was unable to
> > find a better match. Let me know if that should be moved.
>
> I think that's fine. From time to time we move them around when better
> classification can be found.
>
> > - If that's a concern, there is code duplication b/w proxy.c [1]  and this
> > cli.c. I am unable to find a file where we can create a utility function.
>
> I'm not seeing anything wrong with what you've done.
>
> > Mostly, the concern is to modify "global.tune.options" whenever
> > "global.close_spread_time" changes.
>
> Ah, good point!
>
> > - I noticed global struct is accessed without any locks, is that like a
> > "known" race condition for these simple operations? I don't primarily
> > develop C code, and this was confusing to me.
>
> You're totally right, and then you're indeed introducing a bug :-)
>
> It happens that the global struct is only changed during startup, hence
> it's single-threaded at this point. After that it's only read. But for
> such rare operations that consist in changing global shared stuff, we
> have a secret weapon. We can temporarily pause all other threads during
> the change, to guarantee exclusive access to everything. This is done
> by issuing: "thread_isolate();" before the critical code, and
> "thread_release();" at the end (i.e. around your manipulation of
> tune.options and close_spread_time. This means you should rearrange
> the parsing function to group the changes together. That's easier than
> it seems if you set an ormask, andnotmask and the new value from a
> local variable. This way you don't have to care about threads.
>
> I was also thinking about something, I wouldn't be surprised if we start
> to see more commands to change some global ones like this. As such I
> think it would be better to call this "set global close-spread-time" so
> that is more commands affecting the global section happen later, they'll
> already be grouped together and will be easier to find.
>
> Thanks!
> Willy



-- 
Cheers,
Abhijeet (https://abhi.host)


0001-MINOR-cli-add-option-to-modify-close-spread-time.patch
Description: Binary data

Re: [PATCH] MINOR: cli: add option to modify close-spread-time

[Willy Tarreau]

Hi Abhijeet,

On Mon, Apr 08, 2024 at 08:11:28PM -0700, Abhijeet Rastogi wrote:
> Hi HAproxy community,
> 
> Let's assume that HAproxy starts with non-zero values for close-spread-time
> and hard-stop-after, and soft-stop is used to initiate the shutdown during
> deployments.
> There are times where we need to modify these values, eg, in case the
> operator needs to restart HAproxy faster, but still use the soft-stop
> workflow.
> 
> So, it will be nice to have the ability to modify these values via CLI.
> 
> RFC:-
> - Does this seem like a fair use-case?
> - If this is good, I can also work on the patch for hard-stop-after.

Yes, I think that both totally make sense from a production perspective.
I hadn't thought about that before, but it's obvious that when you've
deployed with a wrong setting or are observing a bad behavior, you'd
really like the old process to quit much faster.

> Patch questions:-
> - I've added reg-tests under a new folder "cli" because I was unable to
> find a better match. Let me know if that should be moved.

I think that's fine. From time to time we move them around when better
classification can be found.

> - If that's a concern, there is code duplication b/w proxy.c [1]  and this
> cli.c. I am unable to find a file where we can create a utility function.

I'm not seeing anything wrong with what you've done.

> Mostly, the concern is to modify "global.tune.options" whenever
> "global.close_spread_time" changes.

Ah, good point!

> - I noticed global struct is accessed without any locks, is that like a
> "known" race condition for these simple operations? I don't primarily
> develop C code, and this was confusing to me.

You're totally right, and then you're indeed introducing a bug :-)

It happens that the global struct is only changed during startup, hence
it's single-threaded at this point. After that it's only read. But for
such rare operations that consist in changing global shared stuff, we
have a secret weapon. We can temporarily pause all other threads during
the change, to guarantee exclusive access to everything. This is done
by issuing: "thread_isolate();" before the critical code, and
"thread_release();" at the end (i.e. around your manipulation of
tune.options and close_spread_time. This means you should rearrange
the parsing function to group the changes together. That's easier than
it seems if you set an ormask, andnotmask and the new value from a
local variable. This way you don't have to care about threads.

I was also thinking about something, I wouldn't be surprised if we start
to see more commands to change some global ones like this. As such I
think it would be better to call this "set global close-spread-time" so
that is more commands affecting the global section happen later, they'll
already be grouped together and will be easier to find.

Thanks!
Willy

Re: Changes in HAProxy 3.0's Makefile and build options

[Илья Шипицин]

сб, 13 апр. 2024 г. в 15:26, Willy Tarreau :

> Hi Tristan,
>
> On Fri, Apr 12, 2024 at 07:38:18AM +, Tristan wrote:
> > Hi Willy,
> >
> > > On 11 Apr 2024, at 18:18, Willy Tarreau  wrote:
> > >
> > > Some distros simply found that stuffing their regular CFLAGS into
> > > DEBUG_CFLAGS or CPU_CFLAGS does the trick most of the time. Others use
> > > other combinations depending on the tricks they figured.
> >
> > Good to know I wasn't alone scratching my head about what came from
> where!
>
> Definitely.
>
> > > After this analysis, I figured that we needed to simplify all this, get
> > > back to what is more natural to use or more commonly found, without
> > > breaking everything for everyone. [...]
> >
> > These are very nice improvements indeed, but I admit that if (at least
> > partially) breaking backwards compatibility was the acceptable choice
> here,
>
> It should almost not break it otherwise it will indicate what changed
> and how to proceed.
>
> > I'd have hoped to see something like cmake rather than a makefile
> > refactoring.
>

I would like to hear real pain behind the idea of moving to cmake.


I suspect such pain exists, for example I had to explicitly specify LUA
options depending on linux distro (it was hard to autodetect
LUA by Makefile, but it is easier for cmake)

maybe we can deal with the pain and improve things using make :)



>
> That's orthogonal. cmake is an alternative to autoconf, not to make,
> you still run "make -j$(nproc)" once cmake is done.
>
> And such mechanisms are really really a pain to deal with, at every
> stage (development and user). They can be justified for ultra-complex
> projects but quite frankly, having to imagine not being able to flip
> an option without rebuilding everything, not having something as simple
> as "V=1" to re-run the failed file and see exactly what was tried,
> having to fight against the config generator all the time etc is really
> a no-go for me. I even remember having stopped using OpenCV long ago
> when it switched from autoconf to cmake because it turned something
> complicated to something out of control that would no longer ever build
> outside of the authors' environment. We could say whatever, like they
> did it wrong or anything else, but the fact is I'm not going to impose
> a similar pain to our users.
>
> In our case, we only need to set a list of files to be built, and pass
> a few -I/-L/-l while leaving the final choice to the user.
>
> > Actually, I'd thought a few times in the past about starting a
> discussion in
> > that direction but figured it would be inconceivable.
> >
> > I don't know how controversial it is, so the main two reasons I mention
> it are:
> > - generally better tooling (and IDE support) out of the box:
> options/flags
> >   discovery and override specifically tends to be quite a bit simpler as
> the
> >   boilerplate and conventions are mostly handled by default
> > - easier to parse final result: both of them look frankly awful, but the
> >   result of cmake setups is often a little easier to parse as it
> encourages a
> >   rather declarative style (can be done in gmake, but it is very much up
> to the
> >   maintainers to be extremely disciplined about it)
>
> The problem with tools like cmake is not to parse the output when it
> works but to figure how to force it to accept that *you* are the one who
> knows when it decides it will not do what you want. While a makefile can
> trivially be overridden or even fixed, good luck for guessing all the
> magic names of cmake variables that are missing and that may help you.
>
> I really do understand why some super complex projects involving git
> submodules and stuff like this would go in that direction, but otherwise
> it's just asking for trouble with little to no benefit.
>
> > Arguably, there's the downside of requiring an extra tool everyone might
> not
> > be deeply familiar with already, and cmake versions matter more than
> gmake
> > ones so I would worry about compatibility for distros of the GCC 4 era,
> but
> > it seemed to me like it's reasonably proven and spread by now to
> consider.
>
> It's not even a matter of compatibility with any gcc version, rather a
> compatibility with what developers are able to write and what users want
> to do if that was not initially imagined by the developers. Have you read
> already about all the horrors faced by users trying to use distcc or ccache
> with cmake, often having to run sed over their cmake files ? Some time ago,
> cmake even implemented yet another magic variable specifically to

Re: Changes in HAProxy 3.0's Makefile and build options

[Willy Tarreau]

Hi Tristan,

On Fri, Apr 12, 2024 at 07:38:18AM +, Tristan wrote:
> Hi Willy,
> 
> > On 11 Apr 2024, at 18:18, Willy Tarreau  wrote:
> > 
> > Some distros simply found that stuffing their regular CFLAGS into
> > DEBUG_CFLAGS or CPU_CFLAGS does the trick most of the time. Others use
> > other combinations depending on the tricks they figured.
> 
> Good to know I wasn't alone scratching my head about what came from where!

Definitely.

> > After this analysis, I figured that we needed to simplify all this, get
> > back to what is more natural to use or more commonly found, without
> > breaking everything for everyone. [...]
> 
> These are very nice improvements indeed, but I admit that if (at least
> partially) breaking backwards compatibility was the acceptable choice here,

It should almost not break it otherwise it will indicate what changed
and how to proceed.

> I'd have hoped to see something like cmake rather than a makefile
> refactoring.

That's orthogonal. cmake is an alternative to autoconf, not to make,
you still run "make -j$(nproc)" once cmake is done.

And such mechanisms are really really a pain to deal with, at every
stage (development and user). They can be justified for ultra-complex
projects but quite frankly, having to imagine not being able to flip
an option without rebuilding everything, not having something as simple
as "V=1" to re-run the failed file and see exactly what was tried,
having to fight against the config generator all the time etc is really
a no-go for me. I even remember having stopped using OpenCV long ago
when it switched from autoconf to cmake because it turned something
complicated to something out of control that would no longer ever build
outside of the authors' environment. We could say whatever, like they
did it wrong or anything else, but the fact is I'm not going to impose
a similar pain to our users.

In our case, we only need to set a list of files to be built, and pass
a few -I/-L/-l while leaving the final choice to the user.

> Actually, I'd thought a few times in the past about starting a discussion in
> that direction but figured it would be inconceivable.
> 
> I don't know how controversial it is, so the main two reasons I mention it 
> are:
> - generally better tooling (and IDE support) out of the box: options/flags
>   discovery and override specifically tends to be quite a bit simpler as the
>   boilerplate and conventions are mostly handled by default
> - easier to parse final result: both of them look frankly awful, but the
>   result of cmake setups is often a little easier to parse as it encourages a
>   rather declarative style (can be done in gmake, but it is very much up to 
> the
>   maintainers to be extremely disciplined about it)

The problem with tools like cmake is not to parse the output when it
works but to figure how to force it to accept that *you* are the one who
knows when it decides it will not do what you want. While a makefile can
trivially be overridden or even fixed, good luck for guessing all the
magic names of cmake variables that are missing and that may help you.

I really do understand why some super complex projects involving git
submodules and stuff like this would go in that direction, but otherwise
it's just asking for trouble with little to no benefit.

> Arguably, there's the downside of requiring an extra tool everyone might not
> be deeply familiar with already, and cmake versions matter more than gmake
> ones so I would worry about compatibility for distros of the GCC 4 era, but
> it seemed to me like it's reasonably proven and spread by now to consider.

It's not even a matter of compatibility with any gcc version, rather a
compatibility with what developers are able to write and what users want
to do if that was not initially imagined by the developers. Have you read
already about all the horrors faced by users trying to use distcc or ccache
with cmake, often having to run sed over their cmake files ? Some time ago,
cmake even implemented yet another magic variable specifically to make this
less painful. And cross-compilation is yet another entire topic. All of
this just ends up in a situation where the build system becomes an entire
project on its own, just for the sake of passing 6 variables to make in
the end :-/  In the case of a regular makefile at least, 100% of the
variables you may have to use are present in the makefile, you don't need
to resort to random guesses by copy-pasting from stackoverflow and see if
they improve your experience.

Long ago I thought that we could possibly support a config file to carry
options between versions, a bit like the .config in the Linux kernel.
Because one of the difficulties that those using autoconf/cmake is to
figure in what form to store a working setup. In practice there's no
magic solution and you end up savin

Re: [PATCH 0/1] CI: revert entropy hack

[Willy Tarreau]

On Sat, Apr 13, 2024 at 09:50:33AM +0200,  ??? wrote:
> It has been resolved on image generation side
> https://github.com/actions/runner-images/issues/9491
> 
> It is no harm to keep it on our side as well, but we can drop it

Perfect, now merged, thank you Ilya!
Willy

Re: [PATCH 0/1] CI: revert entropy hack

[Илья Шипицин]

It has been resolved on image generation side
https://github.com/actions/runner-images/issues/9491

It is no harm to keep it on our side as well, but we can drop it

On Fri, Apr 12, 2024, 18:55 Willy Tarreau  wrote:

> On Fri, Apr 12, 2024 at 12:42:51PM +0200,  ??? wrote:
> > ping :)
>
> Ah thanks for the reminder. I noticed it a few days ago and I wanted to
> ask you to please include a commit message explaining why it's no longer
> necessary. We don't need much, just to understand the rationale for the
> removal.
>
> If you just send me one or two human-readable sentences that can be
> copy-pasted in the message, I'm willing to do it myself to save you
> from resending.
>
> Thanks,
> Willy
>

Re: [PR] DOC: management: fix typos

[Willy Tarreau]

On Fri, Apr 12, 2024 at 10:23:02AM +, PR Bot wrote:
> Dear list!
> 
> Author: Andrey Lebedev 
> Number of patches: 1
> 
> This is an automated relay of the Github pull request:
>DOC: management: fix typos
(...)

Now merged, thank you Andrey!
Willy

Re: [PATCH 0/1] CI: revert entropy hack

[Willy Tarreau]

On Fri, Apr 12, 2024 at 12:42:51PM +0200,  ??? wrote:
> ping :)

Ah thanks for the reminder. I noticed it a few days ago and I wanted to
ask you to please include a commit message explaining why it's no longer
necessary. We don't need much, just to understand the rationale for the
removal.

If you just send me one or two human-readable sentences that can be
copy-pasted in the message, I'm willing to do it myself to save you
from resending.

Thanks,
Willy

Re: [PATCH] MINOR: config: rhttp: Downgrade error on attach-srv name parsing

[William Manley]

On Fri, Apr 12, 2024, at 4:01 PM, Amaury Denoyelle wrote:
> I have a doubt though, will this kind of configuration really works ?  I
> though that for the moment if name parameter is specified, it is
> mandatory to use a server with SSL+SNI.

It may be mandatory according to the RFC, but I'm not using it that way.

Usually it's RHTTP over SSL, and the incoming connection identifies itself 
securely using the SSL DN.

The way I'm using it is RHTTP over HTTP CONNECT - and I'm validating the 
connection using the headers that came with the HTTP CONNECT.  I have tcp 
server block that strips the HTTP CONNECT header and adds PROXY header instead 
with the connection pool name sent through using unique-id:

listen connect_terminate
mode tcp
bind ...
tcp-request inspect-delay 5s
tcp-request content lua.terminate_http_connect

# This allows us to send the hostname over the PROXY protocol:
unique-id-format "%[var(txn.req_header.x_hostname)]"
server srv 127.0.0.1:8001 send-proxy-v2 proxy-v2-options unique-id

Then I use that unique id when adding the connection to the connection pool:

frontend add_to_http_pool
mode http
bind 127.0.0.1:8001 proto h2 accept-proxy
tcp-request session attach-srv rhttp_frontend/srv name 
fc_pp_unique_id

It's a little roundabout (and this is the simplified version) but quite 
capable. I plan to use a similar technique to route multiple requests to 
different hostnames down the same RHTTP connection too.  In that case I'll not 
be using sni req.hdr(host) either - but I haven't got that far yet.

Thanks

Will
---
William Manley
Stb-tester.com

Stb-tester.com Ltd is a company registered in England and Wales.
Registered number: 08800454. Registered office: 13B The Vale,
London, W3 7SH, United Kingdom (This is not a remittance address.)

Re: [PATCH] MINOR: config: rhttp: Downgrade error on attach-srv name parsing

[Willy Tarreau]

On Fri, Apr 12, 2024 at 05:01:07PM +0200, Amaury Denoyelle wrote:
> On Fri, Apr 12, 2024 at 03:37:56PM +0200, Willy Tarreau wrote:
> > Hi!
> > On Fri, Apr 12, 2024 at 02:29:30PM +0100, William Manley wrote:
> > > An attach-srv config line usually looks like this:
> > > > tcp-request session attach-srv be/srv name ssl_c_s_dn(CN)
> > > > The name is a key that is used when looking up connections in the
> > > connection pool.  Without this patch you'd get an error if you passed
> > > anything other than "ssl_c_s_dn(CN)" as the name expression.  Now you can
> > > pass arbitrary expressions and it will just warn you if you aren't
> > > producing a configuration that is RFC compliant.
> > > > I'm doing this as I want to use `fc_pp_unique_id` as the name.
> > > ---
> > >  src/tcp_act.c | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > > diff --git a/src/tcp_act.c b/src/tcp_act.c
> > > index a88fab4af..4d2a56c67 100644
> > > --- a/src/tcp_act.c
> > > +++ b/src/tcp_act.c
> > > @@ -522,8 +522,7 @@ static int tcp_check_attach_srv(struct act_rule 
> > > *rule, struct proxy *px, char **
> > >  
> > >   if ((rule->arg.attach_srv.name && (!srv->use_ssl || !srv->sni_expr)) ||
> > >   (!rule->arg.attach_srv.name && srv->use_ssl && srv->sni_expr)) {
> > > - memprintf(err, "attach-srv rule: connection will never be used; 
> > > either specify name argument in conjunction with defined SSL SNI on 
> > > targeted server or none of these");
> > > - return 0;
> > > + ha_warning("attach-srv rule: connection may never be used; 
> > > usually name argument is defined SSL SNI on targeted server or none of 
> > > these");
> > Well, I consider that any valid (and useful) configuration must be
> > writable without a warning. So if you have a valid use case with a
> > different expression, here you still have no way to express it without
> > the warning. In this case I'd suggest to use ha_diag_warning() instead,
> > it will only report this when starting with -dD (config diagnostic mode).
> 
> I have a doubt though, will this kind of configuration really works ?  I
> though that for the moment if name parameter is specified, it is
> mandatory to use a server with SSL+SNI.

If we receive the traffic with SSL already stripped by a front haproxy
and all the info presented in the proxy protocol, I think it should still
work. I must confess that it's blowing my mind a little bit to imagine
all these layers, but I tend to think that could be valid.

Willy

Re: [PATCH] MINOR: config: rhttp: Downgrade error on attach-srv name parsing

[William Manley]

On Fri, Apr 12, 2024, at 2:37 PM, Willy Tarreau wrote:
> Well, I consider that any valid (and useful) configuration must be
> writable without a warning. So if you have a valid use case with a
> different expression, here you still have no way to express it without
> the warning. In this case I'd suggest to use ha_diag_warning() instead,
> it will only report this when starting with -dD (config diagnostic mode).

Thanks for taking a look.  I've taken a slightly different approach now that I 
understand the warning better.  Instead I've removed the requirement for SSL, 
but I keep the requirement that either name and sni are specified or neither 
are.

This way it won't warn or error with my configuration, nor with the usual one, 
but it will still error if there is a mismatch between name and sni.

Thanks

Will
---
William Manley
Stb-tester.com

Stb-tester.com Ltd is a company registered in England and Wales.
Registered number: 08800454. Registered office: 13B The Vale,
London, W3 7SH, United Kingdom (This is not a remittance address.)

Re: [PATCH] MINOR: config: rhttp: Downgrade error on attach-srv name parsing

[Amaury Denoyelle]

On Fri, Apr 12, 2024 at 03:37:56PM +0200, Willy Tarreau wrote:
> Hi!
> On Fri, Apr 12, 2024 at 02:29:30PM +0100, William Manley wrote:
> > An attach-srv config line usually looks like this:
> > > tcp-request session attach-srv be/srv name ssl_c_s_dn(CN)
> > > The name is a key that is used when looking up connections in the
> > connection pool.  Without this patch you'd get an error if you passed
> > anything other than "ssl_c_s_dn(CN)" as the name expression.  Now you can
> > pass arbitrary expressions and it will just warn you if you aren't
> > producing a configuration that is RFC compliant.
> > > I'm doing this as I want to use `fc_pp_unique_id` as the name.
> > ---
> >  src/tcp_act.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > diff --git a/src/tcp_act.c b/src/tcp_act.c
> > index a88fab4af..4d2a56c67 100644
> > --- a/src/tcp_act.c
> > +++ b/src/tcp_act.c
> > @@ -522,8 +522,7 @@ static int tcp_check_attach_srv(struct act_rule *rule, 
> > struct proxy *px, char **
> >  
> > if ((rule->arg.attach_srv.name && (!srv->use_ssl || !srv->sni_expr)) ||
> > (!rule->arg.attach_srv.name && srv->use_ssl && srv->sni_expr)) {
> > -   memprintf(err, "attach-srv rule: connection will never be used; 
> > either specify name argument in conjunction with defined SSL SNI on 
> > targeted server or none of these");
> > -   return 0;
> > +   ha_warning("attach-srv rule: connection may never be used; 
> > usually name argument is defined SSL SNI on targeted server or none of 
> > these");
> Well, I consider that any valid (and useful) configuration must be
> writable without a warning. So if you have a valid use case with a
> different expression, here you still have no way to express it without
> the warning. In this case I'd suggest to use ha_diag_warning() instead,
> it will only report this when starting with -dD (config diagnostic mode).

I have a doubt though, will this kind of configuration really works ?  I
though that for the moment if name parameter is specified, it is
mandatory to use a server with SSL+SNI.

-- 
Amaury Denoyelle

Re: [PATCH] MINOR: config: rhttp: Downgrade error on attach-srv name parsing

[Willy Tarreau]

Hi!

On Fri, Apr 12, 2024 at 02:29:30PM +0100, William Manley wrote:
> An attach-srv config line usually looks like this:
> 
> tcp-request session attach-srv be/srv name ssl_c_s_dn(CN)
> 
> The name is a key that is used when looking up connections in the
> connection pool.  Without this patch you'd get an error if you passed
> anything other than "ssl_c_s_dn(CN)" as the name expression.  Now you can
> pass arbitrary expressions and it will just warn you if you aren't
> producing a configuration that is RFC compliant.
> 
> I'm doing this as I want to use `fc_pp_unique_id` as the name.
> ---
>  src/tcp_act.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/src/tcp_act.c b/src/tcp_act.c
> index a88fab4af..4d2a56c67 100644
> --- a/src/tcp_act.c
> +++ b/src/tcp_act.c
> @@ -522,8 +522,7 @@ static int tcp_check_attach_srv(struct act_rule *rule, 
> struct proxy *px, char **
>  
>   if ((rule->arg.attach_srv.name && (!srv->use_ssl || !srv->sni_expr)) ||
>   (!rule->arg.attach_srv.name && srv->use_ssl && srv->sni_expr)) {
> - memprintf(err, "attach-srv rule: connection will never be used; 
> either specify name argument in conjunction with defined SSL SNI on targeted 
> server or none of these");
> - return 0;
> + ha_warning("attach-srv rule: connection may never be used; 
> usually name argument is defined SSL SNI on targeted server or none of 
> these");

Well, I consider that any valid (and useful) configuration must be
writable without a warning. So if you have a valid use case with a
different expression, here you still have no way to express it without
the warning. In this case I'd suggest to use ha_diag_warning() instead,
it will only report this when starting with -dD (config diagnostic mode).

Willy

Re: [PATCH 0/1] CI: revert entropy hack

[Илья Шипицин]

ping :)

сб, 6 апр. 2024 г. в 15:38, Ilya Shipitsin :

> hack introduced in  3a0fc8641b1549b00cd3125107545b6879677801 might be
> reverted
>
> Ilya Shipitsin (1):
>   CI: revert kernel entropy introduced in
> 3a0fc8641b1549b00cd3125107545b6879677801
>
>  .github/workflows/vtest.yml | 11 ---
>  1 file changed, 11 deletions(-)
>
> --
> 2.44.0
>
>

Re: Changes in HAProxy 3.0's Makefile and build options

[Tristan]

Hi Willy,

> On 11 Apr 2024, at 18:18, Willy Tarreau  wrote:
> 
> Some distros simply found that stuffing their regular CFLAGS into
> DEBUG_CFLAGS or CPU_CFLAGS does the trick most of the time. Others use
> other combinations depending on the tricks they figured.

Good to know I wasn’t alone scratching my head about what came from where!

> After this analysis, I figured that we needed to simplify all this, get
> back to what is more natural to use or more commonly found, without
> breaking everything for everyone. […]

These are very nice improvements indeed, but I admit that if (at least 
partially) breaking backwards compatibility was the acceptable choice here, I’d 
have hoped to see something like cmake rather than a makefile refactoring.

Actually, I’d thought a few times in the past about starting a discussion in 
that direction but figured it would be inconceivable.

I don’t know how controversial it is, so the main two reasons I mention it are:
- generally better tooling (and IDE support) out of the box: options/flags 
discovery and override specifically tends to be quite a bit simpler as the 
boilerplate and conventions are mostly handled by default
- easier to parse final result: both of them look frankly awful, but the result 
of cmake setups is often a little easier to parse as it encourages a rather 
declarative style (can be done in gmake, but it is very much up to the 
maintainers to be extremely disciplined about it)

Arguably, there’s the downside of requiring an extra tool everyone might not be 
deeply familiar with already, and cmake versions matter more than gmake ones so 
I would worry about compatibility for distros of the GCC 4 era, but it seemed 
to me like it’s reasonably proven and spread by now to consider.

Tristan

Re: Changes in HAProxy 3.0's Makefile and build options

[William Lallemand]

On Thu, Apr 11, 2024 at 11:43:14PM +0200, Dinko Korunic wrote:
> Subject: Re: Changes in HAProxy 3.0's Makefile and build options
> 
> > On 11.04.2024., at 21:32, William Lallemand  wrote:
> > 
> > If I remember correctly github actions VMs only had 2 vCPU in the past,
> > I think they upgraded to 4 vCPU last year but I can't find anything in
> > their documentation.
> 
> Hi William,
> 
> GitHub runners Instance sizes for public repositories are now as you said, 4 
> vCPU / 16 GB RAM:
> 
> https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners
> 
> 
> Kind regards,
> D.
> 

Indeed, thanks for confirming! I didn't know there was a difference with
private repositories which are still using 2 vCPU, that explains a lot.

-- 
William Lallemand

Re: Changes in HAProxy 3.0's Makefile and build options

[Dinko Korunic]

> On 11.04.2024., at 21:32, William Lallemand  wrote:
> 
> If I remember correctly github actions VMs only had 2 vCPU in the past,
> I think they upgraded to 4 vCPU last year but I can't find anything in
> their documentation.

Hi William,

GitHub runners Instance sizes for public repositories are now as you said, 4 
vCPU / 16 GB RAM:

https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners


Kind regards,
D.

-- 
Dinko Korunic   ** Standard disclaimer applies **
Sent from OSF1 osf1v4b V4.0 564 alpha

Re: Changes in HAProxy 3.0's Makefile and build options

[William Lallemand]

On Thu, Apr 11, 2024 at 09:04:51PM +0200, Willy Tarreau wrote:
> Subject: Re: Changes in HAProxy 3.0's Makefile and build options
> Hi Ilya,
> 
> On Thu, Apr 11, 2024 at 08:27:39PM +0200,  ??? wrote:
> > do you know maybe how this was supposed to work ?
> > haproxy/Makefile at master · haproxy/haproxy (github.com)
> > <https://github.com/haproxy/haproxy/blob/master/Makefile#L499>
> 
> That's this:
> 
>   ifneq ($(shell $(CC) $(CFLAGS) -dM -E -xc - /dev/null | grep 
> -c 'LOCK_FREE.*1'),0)
> USE_LIBATOMIC   = implicit
>   endif
> 
> It calls the compiler with the known flags and checks if for this arch,
> it's configured to require libatomic.
> 
> > I had to enable atomic explicitly despite it was supposed to be detected on
> > the fly (I haven't had a deeper look yet)
> > 
> > haproxy/.github/workflows/fedora-rawhide.yml at master · haproxy/haproxy
> > <https://github.com/haproxy/haproxy/blob/master/.github/workflows/fedora-rawhide.yml#L17-L18>
> 
> I honestly don't know why it's not working, it could be useful to achive
> the output of this command, either by calling it directly or for example
> by inserting "tee /tmp/log |" before grep. If you have a copy of the linker
> erorr you got without the lib, maybe it will reveal something. But honestly
> this is a perfect example of the reasons why I prefer generic makefiles to
> automatic detection: you see that you need libatomic, you add it, done. No
> need to patch a configure to mask an error due to an unhandled special case
> etc. The test above was mostly meant to ease the build for the most common
> cases (and for me till now it has always worked, on various systems and
> hardware), but I wouldn't mind too much.
> 
> In fact you could simplify your build script by just passing
> USE_LIBATOMIC=1 to enable it.
> 
> BTW, is there a reason for "make -j3" in the build script ? Limiting oneself
> to 3 build processes when modern machines rarely have less than 8 cores is
> a bit of a waste of time, especially if every other package does the same
> in the distro! I'd just do "make -j$(nproc)" as usual there.
> 

If I remember correctly github actions VMs only had 2 vCPU in the past,
I think they upgraded to 4 vCPU last year but I can't find anything in
their documentation.

-- 
William Lallemand

Re: Changes in HAProxy 3.0's Makefile and build options

[Willy Tarreau]

Hi Ilya,

On Thu, Apr 11, 2024 at 08:27:39PM +0200,  ??? wrote:
> do you know maybe how this was supposed to work ?
> haproxy/Makefile at master · haproxy/haproxy (github.com)
> 

That's this:

  ifneq ($(shell $(CC) $(CFLAGS) -dM -E -xc - /dev/null | grep -c 
'LOCK_FREE.*1'),0)
USE_LIBATOMIC   = implicit
  endif

It calls the compiler with the known flags and checks if for this arch,
it's configured to require libatomic.

> I had to enable atomic explicitly despite it was supposed to be detected on
> the fly (I haven't had a deeper look yet)
> 
> haproxy/.github/workflows/fedora-rawhide.yml at master · haproxy/haproxy
> 

I honestly don't know why it's not working, it could be useful to achive
the output of this command, either by calling it directly or for example
by inserting "tee /tmp/log |" before grep. If you have a copy of the linker
erorr you got without the lib, maybe it will reveal something. But honestly
this is a perfect example of the reasons why I prefer generic makefiles to
automatic detection: you see that you need libatomic, you add it, done. No
need to patch a configure to mask an error due to an unhandled special case
etc. The test above was mostly meant to ease the build for the most common
cases (and for me till now it has always worked, on various systems and
hardware), but I wouldn't mind too much.

In fact you could simplify your build script by just passing
USE_LIBATOMIC=1 to enable it.

BTW, is there a reason for "make -j3" in the build script ? Limiting oneself
to 3 build processes when modern machines rarely have less than 8 cores is
a bit of a waste of time, especially if every other package does the same
in the distro! I'd just do "make -j$(nproc)" as usual there.

Cheers,
Willy

Re: Changes in HAProxy 3.0's Makefile and build options

[Илья Шипицин]

чт, 11 апр. 2024 г. в 19:18, Willy Tarreau :

> Hi all,
>
> after all the time where we've all been complaining about the difficulty
> to adjust CFLAGS during the build, I could tackle the problem for a first
> step in the right direction.
>
> First, let's start with a bit of history to explain the situation and why
> it was bad. Originally, a trivial makefile with very simple rules and a
> single object to build (haproxy.o) had just CFLAGS set to "-O2 -g" or
> something like that, and that was perfect. It was possible to just pass a
> different CFLAGS value on the "make" command line and be done with it.
>
> Options started to pile up, but in a way that remained manageable for a
> long time (e.g. add PCRE support, later dlmalloc), so CFLAGS was still the
> only thing to override if needed. With 32/64 bit variants and so on, we
> started to feel the need to split those CFLAGS into multiple pieces for
> more flexibility. But these pieces were still aggregated into CFLAGS so
> that those used to overriding it were still happy. This situation forced
> us not to break these precious CFLAGS that some users were relying on.
>
> And that went like this for a long time, though the definition of this
> CFLAGS variable became more and more complicated by inheriting from some
> automatic options. For example, in 3.0-dev7, CFLAGS is initialized to
> "$(ARCH_FLAGS) $(CPU_CFLAGS) $(DEBUG_CFLAGS) $(SPEC_CFLAGS)", i.e. it
> concatenates 4 variables (in apparence). The 4th one (SPEC_CFLAGS) is
> already a concatenation of a fixed one (WARN_CFLAGS) and a series of
> automatically detected ones (the rest of it). ARCH_FLAGS is set from a
> a fixed list of presets depending on the ARCH variable, and CPU_CFLAGS
> is set from a list of presets depending on the CPU variable. And the
> most beautiful is that CFLAGS alone is no longer sufficient since some
> of the USE_* options append their own values behind it, and we complete
> with $(TARGET_CFLAGS) $(SMALL_OPTS) $(DEFINE).
>
> Yeah I know that's ugly. We all know it. Whenever someone asks me "how can
> I enable -fsanitize=address because I'd like to run ASAN", I respond "hmmm
> it depends what options you already use and which ones area easiest to
> hack".
>
> Some distros simply found that stuffing their regular CFLAGS into
> DEBUG_CFLAGS or CPU_CFLAGS does the trick most of the time. Others use
> other combinations depending on the tricks they figured.
>
> After this analysis, I figured that we needed to simplify all this, get
> back to what is more natural to use or more commonly found, without
> breaking everything for everyone. What is certain however in the current
> situation, is that nobody is overriding CFLAGS since it's too rich, too
> complex and too unpredictable. So it was really time to reset it.
>
> Thus here's what was done:
>   - CFLAGS is now empty by default and can be passed some build options
> that are appended at the end of the automatic flags. This means that
> -Os, -ggdb3, -Wfoobar, -Wno-foobar, -I../secret-place/include etc
> will properly be honored without having to trick anything anymore.
> Thus for package maintainers, building with CFLAGS="$DISTRO_CFLAGS"
> should just get the work done.
>
>   - LDFLAGS is now empty by default and can be passed some build options
> that are prepended to the list of linker options.
>
>   - ARCH_FLAGS now defaults to -g and is passed to both the compiler and
> the linker. It may be overridden to change the word size (-m32/-m64),
> enable alternate debugging formats (-ggdb3), enable LTO (-flto),
> ASAN (-fsanitize=address) etc. It's in fact for flags that must be
> consistent between the compiler and the linker. It was not renamed
> since it was already there and used quite a bit already.
>
>   - CPU_CFLAGS was preserved for ease of porting but is empty by default.
> Some may find it convenient to pass their -march, -mtune etc there.
>
>   - CPU and ARCH are gone. Let's face it, only 2 of them were usable and
> no single maintainer will be crazy enough to use these options here
> and resort to another approach for other archs. However the previous
> values are mentioned in the doc as a hint about what's known to work
> well.
>
>   - OPT_CFLAGS was created with "-O2" and nothing else. As developers,
> we spend our time juggling between -O2 and -O0 depending on whether
> we're testing or debugging. Some embedded distros also have options
> to pass -O2 or -Os to choose between fast and small, and that may
> fit very well there.
>
>   - STD_CFLAGS contains a few flags related to standards compliance.
> It is currently set to -fwrapv, -fno-strict-overflow and/or empty
> depending on what the compiler prefers. It's important not to touch
> it unless you know exactly what you're doing, and previously these
> options used to be lost by accident when overriding other ones.
>
>   - WARN_CFLAGS is now set to the list of warnings to 

Re: [PATCH] BUG/MINOR: server: fix slowstart behavior

[Willy Tarreau]

Hi Damien,

On Tue, Apr 09, 2024 at 03:37:07PM +, Damien Claisse wrote:
> We observed that a dynamic server which health check is down for longer
> than slowstart delay at startup doesn't trigger the warmup phase, it
> receives full traffic immediately. This has been confirmed by checking
> haproxy UI, weight is immediately the full one (e.g. 75/75), without any
> throttle applied. Further tests showed that it was similar if it was in
> maintenance, and even when entering a down or maintenance state after
> being up.
> Another issue is that if the server is down for less time than
> slowstart, when it comes back up, it briefly has a much higher weight
> than expected for a slowstart.
> 
> An easy way to reproduce is to do the following:
> - Add a server with e.g. a 20s slowstart and a weight of 10 in config
>   file
> - Put it in maintenance using CLI (set server be1/srv1 state maint)
> - Wait more than 20s, enable it again (set server be1/srv1 state ready)
> - Observe UI, weight will show 10/10 immediately.
> If server was down for less than 20s, you'd briefly see a weight and
> throttle value that is inconsistent, e.g. 50% throttle value and a
> weight of 5 if server comes back up after 10s before going back to
> 6% after a second or two.
> 
> Code analysis shows that the logic in server_recalc_eweight stops the
> warmup task by setting server's next state to SRV_ST_RUNNING if it
> didn't change state for longer than the slowstart duration, regardless
> of its current state. As a consequence, a server being down or disabled
> for longer than the slowstart duration will never enter the warmup phase
> when it will be up again.
> 
> Regarding the weight when server comes back up, issue is that even if
> the server is down, we still compute its next weight as if it was up,
> hence when it comes back up, it can briefly have a much higher weight
> than expected during slowstart, until the warmup task is called again
> after last_change is updated.
> 
> This patch aims to fix both issues.
(...)

You analysis makes a lot of sense, and I'm not much surprised that this
has been revealed by dynamic servers, because the startup sequence before
them was very stable and well established. So for sure if certain parts
start in a different order it can have such visible effects. Good catch!

It's now merged, thank you!
Willy

Re: [PR] Add destination ip as source ip

[Willy Tarreau]

On Wed, Apr 10, 2024 at 03:28:06PM +0200, Christopher Faulet wrote:
> Hi,
> 
> Thanks. I have few comments.
> 
> First, your commit message must follow rules of CONTRIBUTING file. The
> commit subject must mention a level (here MINOR) and a scope (here
> connection). Then a commit message must be provided with details on the
> patch. You should describe what you want to achieve with this feature, how
> and when it should be used...
> 
> Then, the documentation must be included in the same patch. This eases the
> maintenance.
> 
> The 'dstip' parameter must also be added in the 'source' server keyword. And
> it must be documented. In the 'source' backend keyword, info about 'client'
> and 'clientip' parameters are provided. The same must be done for 'dstip'.
> 
> In cfg_parse_listen(), 'dst' is used instead of 'dstip' in the error message
> and the wrong flag is set ( CO_SRC_TPROXY_CIP instead of .
> CO_SRC_TPROXY_DIP).
> 
> Finally, I didn't checked deeply, but CO_SRC_TPROXY_CIP is used in
> proto_tcp.c and proto_quic.c files. I guess something must also be added
> here.
> 
> I have also a question. For completeness, could the 'dst' parameter be useful 
> ?

Actually I would *really* prefer that we support an expression here
rather than adding yet another exception to the parsing.

We already took great care to support "hdr_ip(name)" that matches the
expression syntax so that we could later use any other expression. With
an expression we could support maps and plenty of other facilities. Our
expressions already have an address output type so we could simply say
that we support client/clientip or an expression of type address. I
can easily imagine how some users would for example hope to map sni to
source IP etc. Given that we already support expressions for "sni", I
don't think that it's very difficult to do the same for "usesrc".

In this case the correct destination parameter for the case above would
be "dst" or even "fc_dst" so that it doesn't get broken by "set-dst".

Willy

Re: [PR] Add destination ip as source ip

[Christopher Faulet]

Hi,

Thanks. I have few comments.

First, your commit message must follow rules of CONTRIBUTING file. The commit 
subject must mention a level (here MINOR) and a scope (here connection). Then a 
commit message must be provided with details on the patch. You should describe 
what you want to achieve with this feature, how and when it should be used...


Then, the documentation must be included in the same patch. This eases the 
maintenance.


The 'dstip' parameter must also be added in the 'source' server keyword. And it 
must be documented. In the 'source' backend keyword, info about 'client' and 
'clientip' parameters are provided. The same must be done for 'dstip'.


In cfg_parse_listen(), 'dst' is used instead of 'dstip' in the error message and 
the wrong flag is set ( CO_SRC_TPROXY_CIP instead of . CO_SRC_TPROXY_DIP).


Finally, I didn't checked deeply, but CO_SRC_TPROXY_CIP is used in proto_tcp.c 
and proto_quic.c files. I guess something must also be added here.


I have also a question. For completeness, could the 'dst' parameter be useful ?

--
Christopher Faulet

Re: haproxy backend server template service discovery questions

[Andrii Ustymenko]

he
reply overrides the table, but sometimes it seems to
just gets merged
with the rest.

2) Each entry from SRV reply will be placed into the
table under
specific se_id. Most of the times that placement won't
change. So, for
the example above the most likely 0a5099e5.addr.consul. and
0a509934.addr.consul. will have se_id 1 and 2
respectively. However
sometimes we have the following scenario:

1. We admistratively disable the server (drain traffic)
with the next
command:

```
echo "set server example/application1 state maint" | nc -U
/var/lib/haproxy/stats
```

the MAINT flag will be added to the record with se_id 1

2. Instance of application goes down and gets
de-registered from consul,
so also evicted from srv replies and out of discovery of
haproxy.

3. Instance of application goes up and gets registered
by consul and
discovered by haproxy, but haproxy allocates different
se_id for it.
Haproxy healthchecks will control the traffic to it in
this case.

4. We will still have se_id 1 with MAINT flag and
application instance
dns record placed into different se_id.

The problem comes that any new discovered record which
get placed into
se_id 1 will never be active until either command:

```
echo "set server example/application1 state ready" | nc -U
/var/lib/haproxy/stats
```

executed or haproxy gets reloaded without state file.
With this pattern
we basically have persistent "records pollution" due to
operations made
directly with control socket.

I am not sure is there anything to do about this. Maybe,
if haproxy
could cache the state not only of se_id but also
associated record with
that and then if that gets changed - re-schedule
healtchecks. Or instead
of integer ids use some hashed ids based on
dns/ip-addresses of
discovered records, in this case binding will happen
exactly in the same
slot.

Thanks in advance!

--

Best regards,

Andrii Ustymenko


-- 


Andrii Ustymenko
Platform Reliability Engineer

office +31 20 240 12 40

Adyen Headquarters
Simon Carmiggeltstraat 6-50, 5th floor
1011 DJ Amsterdam, The Netherlands




Adyen <https://www.adyen.com>

-- 


Andrii Ustymenko
Platform Reliability Engineer

office +31 20 240 12 40

Adyen Headquarters
Simon Carmiggeltstraat 6-50, 5th floor
1011 DJ Amsterdam, The Netherlands




Adyen <https://www.adyen.com>


--

Andrii Ustymenko
Platform Reliability Engineer

office +31 20 240 12 40

Adyen Headquarters
Simon Carmiggeltstraat 6-50, 5th floor
1011 DJ Amsterdam, The Netherlands




Adyen <https://www.adyen.com>

Re: haproxy backend server template service discovery questions

[Илья Шипицин]

 with the next
>>> command:
>>>
>>> ```
>>> echo "set server example/application1 state maint" | nc -U
>>> /var/lib/haproxy/stats
>>> ```
>>>
>>> the MAINT flag will be added to the record with se_id 1
>>>
>>> 2. Instance of application goes down and gets de-registered from consul,
>>> so also evicted from srv replies and out of discovery of haproxy.
>>>
>>> 3. Instance of application goes up and gets registered by consul and
>>> discovered by haproxy, but haproxy allocates different se_id for it.
>>> Haproxy healthchecks will control the traffic to it in this case.
>>>
>>> 4. We will still have se_id 1 with MAINT flag and application instance
>>> dns record placed into different se_id.
>>>
>>> The problem comes that any new discovered record which get placed into
>>> se_id 1 will never be active until either command:
>>>
>>> ```
>>> echo "set server example/application1 state ready" | nc -U
>>> /var/lib/haproxy/stats
>>> ```
>>>
>>> executed or haproxy gets reloaded without state file. With this pattern
>>> we basically have persistent "records pollution" due to operations made
>>> directly with control socket.
>>>
>>> I am not sure is there anything to do about this. Maybe, if haproxy
>>> could cache the state not only of se_id but also associated record with
>>> that and then if that gets changed - re-schedule healtchecks. Or instead
>>> of integer ids use some hashed ids based on dns/ip-addresses of
>>> discovered records, in this case binding will happen exactly in the same
>>> slot.
>>>
>>> Thanks in advance!
>>>
>>> --
>>>
>>> Best regards,
>>>
>>> Andrii Ustymenko
>>>
>>>
>>> --
>>
>> Andrii Ustymenko
>> Platform Reliability Engineer
>>
>> office +31 20 240 12 40
>>
>> Adyen Headquarters
>> Simon Carmiggeltstraat 6-50, 5th floor
>> 1011 DJ Amsterdam, The Netherlands
>>
>>
>>
>>
>> [image: Adyen] <https://www.adyen.com>
>>
> --
>
> Andrii Ustymenko
> Platform Reliability Engineer
>
> office +31 20 240 12 40
>
> Adyen Headquarters
> Simon Carmiggeltstraat 6-50, 5th floor
> 1011 DJ Amsterdam, The Netherlands
>
>
>
>
> [image: Adyen] <https://www.adyen.com>
>

Re: haproxy backend server template service discovery questions

[Andrii Ustymenko]

  3. Instance of application goes up and gets registered by
consul and
discovered by haproxy, but haproxy allocates different se_id
for it.
Haproxy healthchecks will control the traffic to it in this case.

4. We will still have se_id 1 with MAINT flag and application
instance
dns record placed into different se_id.

The problem comes that any new discovered record which get
placed into
se_id 1 will never be active until either command:

```
echo "set server example/application1 state ready" | nc -U
/var/lib/haproxy/stats
```

executed or haproxy gets reloaded without state file. With
this pattern
we basically have persistent "records pollution" due to
operations made
directly with control socket.

I am not sure is there anything to do about this. Maybe, if
haproxy
could cache the state not only of se_id but also associated
record with
that and then if that gets changed - re-schedule healtchecks.
Or instead
of integer ids use some hashed ids based on dns/ip-addresses of
discovered records, in this case binding will happen exactly
in the same
slot.

Thanks in advance!

--

Best regards,

Andrii Ustymenko


-- 


Andrii Ustymenko
Platform Reliability Engineer

office +31 20 240 12 40

Adyen Headquarters
Simon Carmiggeltstraat 6-50, 5th floor
1011 DJ Amsterdam, The Netherlands




Adyen <https://www.adyen.com>


--

Andrii Ustymenko
Platform Reliability Engineer

office +31 20 240 12 40

Adyen Headquarters
Simon Carmiggeltstraat 6-50, 5th floor
1011 DJ Amsterdam, The Netherlands




Adyen <https://www.adyen.com>

Re: haproxy backend server template service discovery questions

[Илья Шипицин]

and particularly your question is "does HAProxy merge all responses or pick
the first one or use some other approach" ?

пн, 8 апр. 2024 г. в 10:23, Andrii Ustymenko :

> I guess indeed it is not a case of consul-template specifically, but more
> of rendered templates and how haproxy maintains it.
> On 06/04/2024 20:15, Илья Шипицин wrote:
>
> Consul template is something done by consul itself, after that
> haproxy.conf is rendered
>
> Do you mean "how haproxy deals with rendered template"?
>
> On Fri, Apr 5, 2024, 15:02 Andrii Ustymenko 
> wrote:
>
>> Dear list!
>>
>> My name is Andrii. I work for Adyen. We are using haproxy as our main
>> software loadbalancer at quite large scale.
>> Af of now our main use-case for backends routing based on
>> server-template and dynamic dns from consul as service discovery. Below
>> is the example of simple backend configuration:
>>
>> ```
>> backend example
>>balance roundrobin
>>server-template application 10 _application._tcp.consul resolvers
>> someresolvers init-addr last,libc,none resolve-opts allow-dup-ip
>> resolve-prefer ipv4 check ssl verify none
>> ```
>>
>> and in global configuration
>>
>> ```
>> resolvers someresolvers
>>nameserver ns1 10.10.10.10:53
>>nameserver ns2 10.10.10.11:53
>> ```
>>
>> As we see haproxy will create internal table for backends with some
>> be_id and be_name=application and allocate 10 records for each server
>> with se_id from 1 to 10. Then those records get populated and updated
>> with the data from resolvers.
>> I would like to understand couple of things with regards to this
>> structure and how it works, which I could not figure out myself from the
>> source code:
>>
>> 1) In tcpdump for dns queries we see that haproxy asynchronously polls
>> all the nameservers simultaneously. For instance:
>>
>> ```
>> 11:06:17.587798 eth2  Out ifindex 4 aa:aa:aa:aa:aa:aa ethertype IPv4
>> (0x0800), length 108: 10.10.10.50.24050 > 10.10.10.10.53: 34307+ [1au]
>> SRV? _application._tcp.consul. (60)
>> 11:06:17.587802 eth2  Out ifindex 4 aa:aa:aa:aa:aa:aa ethertype IPv4
>> (0x0800), length 108: 10.10.10.50.63155 > 10.10.10.11.53: 34307+ [1au]
>> SRV? _application._tcp.consul. (60)
>> 11:06:17.588097 eth2  In  ifindex 4 ff:ff:ff:ff:ff:ff ethertype IPv4
>> (0x0800), length 205: 10.10.10.10.53 > 10.10.10.50.24050: 2194 2/0/1 SRV
>> 0a5099e5.addr.consul.:25340 1 1, SRV 0a509934.addr.consul.:26010 1 1 (157)
>> 11:06:17.588097 eth2  In  ifindex 4 ff:ff:ff:ff:ff:ff ethertype IPv4
>> (0x0800), length 205: 10.10.10.11.53 > 10.10.10.50.63155: 2194 2/0/1 SRV
>> 0a5099e5.addr.consul.:25340 1 1, SRV 0a509934.addr.consul.:26010 1 1 (157)
>> ```
>>
>> Both nameservers reply with the same response. But what if they are out
>> of sync? Let's say one says: server1, server2 and the second one says
>> server2, server3? So far testing this locally - I see sometimes the
>> reply overrides the table, but sometimes it seems to just gets merged
>> with the rest.
>>
>> 2) Each entry from SRV reply will be placed into the table under
>> specific se_id. Most of the times that placement won't change. So, for
>> the example above the most likely 0a5099e5.addr.consul. and
>> 0a509934.addr.consul. will have se_id 1 and 2 respectively. However
>> sometimes we have the following scenario:
>>
>> 1. We admistratively disable the server (drain traffic) with the next
>> command:
>>
>> ```
>> echo "set server example/application1 state maint" | nc -U
>> /var/lib/haproxy/stats
>> ```
>>
>> the MAINT flag will be added to the record with se_id 1
>>
>> 2. Instance of application goes down and gets de-registered from consul,
>> so also evicted from srv replies and out of discovery of haproxy.
>>
>> 3. Instance of application goes up and gets registered by consul and
>> discovered by haproxy, but haproxy allocates different se_id for it.
>> Haproxy healthchecks will control the traffic to it in this case.
>>
>> 4. We will still have se_id 1 with MAINT flag and application instance
>> dns record placed into different se_id.
>>
>> The problem comes that any new discovered record which get placed into
>> se_id 1 will never be active until either command:
>>
>> ```
>> echo "set server example/application1 state ready" | nc -U
>> /var/lib/haproxy/stats
>> ```
>>
>> executed or haproxy gets reloaded without state file. With this pattern
>> we ba

Re: haproxy backend server template service discovery questions

[Andrii Ustymenko]

I guess indeed it is not a case of consul-template specifically, but 
more of rendered templates and how haproxy maintains it.


On 06/04/2024 20:15, Илья Шипицин wrote:
Consul template is something done by consul itself, after that 
haproxy.conf is rendered


Do you mean "how haproxy deals with rendered template"?

On Fri, Apr 5, 2024, 15:02 Andrii Ustymenko 
 wrote:


Dear list!

My name is Andrii. I work for Adyen. We are using haproxy as our main
software loadbalancer at quite large scale.
Af of now our main use-case for backends routing based on
server-template and dynamic dns from consul as service discovery.
Below
is the example of simple backend configuration:

```
backend example
   balance roundrobin
   server-template application 10 _application._tcp.consul resolvers
someresolvers init-addr last,libc,none resolve-opts allow-dup-ip
resolve-prefer ipv4 check ssl verify none
```

and in global configuration

```
resolvers someresolvers
   nameserver ns1 10.10.10.10:53 <http://10.10.10.10:53>
   nameserver ns2 10.10.10.11:53 <http://10.10.10.11:53>
```

As we see haproxy will create internal table for backends with some
be_id and be_name=application and allocate 10 records for each server
with se_id from 1 to 10. Then those records get populated and updated
with the data from resolvers.
I would like to understand couple of things with regards to this
structure and how it works, which I could not figure out myself
from the
source code:

1) In tcpdump for dns queries we see that haproxy asynchronously
polls
all the nameservers simultaneously. For instance:

```
11:06:17.587798 eth2  Out ifindex 4 aa:aa:aa:aa:aa:aa ethertype IPv4
(0x0800), length 108: 10.10.10.50.24050 > 10.10.10.10.53: 34307+
[1au]
SRV? _application._tcp.consul. (60)
11:06:17.587802 eth2  Out ifindex 4 aa:aa:aa:aa:aa:aa ethertype IPv4
(0x0800), length 108: 10.10.10.50.63155 > 10.10.10.11.53: 34307+
[1au]
SRV? _application._tcp.consul. (60)
11:06:17.588097 eth2  In  ifindex 4 ff:ff:ff:ff:ff:ff ethertype IPv4
(0x0800), length 205: 10.10.10.10.53 > 10.10.10.50.24050: 2194
2/0/1 SRV
0a5099e5.addr.consul.:25340 1 1, SRV 0a509934.addr.consul.:26010 1
1 (157)
11:06:17.588097 eth2  In  ifindex 4 ff:ff:ff:ff:ff:ff ethertype IPv4
(0x0800), length 205: 10.10.10.11.53 > 10.10.10.50.63155: 2194
2/0/1 SRV
0a5099e5.addr.consul.:25340 1 1, SRV 0a509934.addr.consul.:26010 1
1 (157)
```

Both nameservers reply with the same response. But what if they
are out
of sync? Let's say one says: server1, server2 and the second one says
server2, server3? So far testing this locally - I see sometimes the
reply overrides the table, but sometimes it seems to just gets merged
with the rest.

2) Each entry from SRV reply will be placed into the table under
specific se_id. Most of the times that placement won't change. So,
for
the example above the most likely 0a5099e5.addr.consul. and
0a509934.addr.consul. will have se_id 1 and 2 respectively. However
sometimes we have the following scenario:

1. We admistratively disable the server (drain traffic) with the next
command:

```
echo "set server example/application1 state maint" | nc -U
/var/lib/haproxy/stats
```

the MAINT flag will be added to the record with se_id 1

2. Instance of application goes down and gets de-registered from
consul,
so also evicted from srv replies and out of discovery of haproxy.

3. Instance of application goes up and gets registered by consul and
discovered by haproxy, but haproxy allocates different se_id for it.
Haproxy healthchecks will control the traffic to it in this case.

4. We will still have se_id 1 with MAINT flag and application
instance
dns record placed into different se_id.

The problem comes that any new discovered record which get placed
into
se_id 1 will never be active until either command:

```
echo "set server example/application1 state ready" | nc -U
/var/lib/haproxy/stats
```

executed or haproxy gets reloaded without state file. With this
pattern
we basically have persistent "records pollution" due to operations
made
directly with control socket.

I am not sure is there anything to do about this. Maybe, if haproxy
could cache the state not only of se_id but also associated record
with
that and then if that gets changed - re-schedule healtchecks. Or
instead
of integer ids use some hashed ids based on dns/ip-addresses of
discovered records, in this case binding will happen exactly in
the same
slot.

Thanks in advance!

--

Best regards,

Andrii Ustymenko



--

Andrii Ustymenko
Platform Reliabi

Re: [ANNOUNCE] haproxy-3.0-dev7

[Willy Tarreau]

Hi Ilya,

On Sun, Apr 07, 2024 at 08:34:18PM +0200,  ??? wrote:
> ??, 6 ???. 2024 ?. ? 17:53, Willy Tarreau :
> >   - a new "guid" keyword was added for servers, listeners and proxies.
> > The purpose will be to make it possible for external APIs to assign
> > a globally unique object identifier to each of them in stats dumps
> > or CLI accesses, and to later reliably recognize a server upon
> > reloads. For now the identifier is not exploited.
> >
> 
> I have a question about the UUID version. it is not specified. Is it UUID
> version 6 ?

It's not a UUID, it's a free string, up to 128 chars long. This way
the API client can use whatever it wants, including a UUID.

Regarding UUIDs, though, I've recently come across UUIDv7 which I found
particularly interesting, and that I think would be nice to implement
in the uuid() sample fetch function before 3.0 is released.

Cheers,
Willy

Re: haproxy backend server template service discovery questions

[Pavlos Parissis]

On Sat, 6 Apr 2024 at 20:17, Илья Шипицин  wrote:
>
> Consul template is something done by consul itself, after that haproxy.conf 
> is rendered
>
> Do you mean "how haproxy deals with rendered template"?
>

He doesn't use that method of discovery, he uses DNS resolvers so
haproxy gets the SRV records with the list of
servers of backends.

Cheers,
Pavlos

Re: haproxy backend server template service discovery questions

[Pavlos Parissis]

On Fri, 5 Apr 2024 at 15:00, Andrii Ustymenko
 wrote:
>
> Dear list!
>
> My name is Andrii. I work for Adyen. We are using haproxy as our main
> software loadbalancer at quite large scale.
> Af of now our main use-case for backends routing based on
> server-template and dynamic dns from consul as service discovery. Below
> is the example of simple backend configuration:
>
> ```
> backend example
>balance roundrobin
>server-template application 10 _application._tcp.consul resolvers
> someresolvers init-addr last,libc,none resolve-opts allow-dup-ip
> resolve-prefer ipv4 check ssl verify none
> ```
>
> and in global configuration
>
> ```
> resolvers someresolvers
>nameserver ns1 10.10.10.10:53
>nameserver ns2 10.10.10.11:53
> ```
>
> As we see haproxy will create internal table for backends with some
> be_id and be_name=application and allocate 10 records for each server
> with se_id from 1 to 10. Then those records get populated and updated
> with the data from resolvers.
> I would like to understand couple of things with regards to this
> structure and how it works, which I could not figure out myself from the
> source code:
>
> 1) In tcpdump for dns queries we see that haproxy asynchronously polls
> all the nameservers simultaneously. For instance:
>
> ```
> 11:06:17.587798 eth2  Out ifindex 4 aa:aa:aa:aa:aa:aa ethertype IPv4
> (0x0800), length 108: 10.10.10.50.24050 > 10.10.10.10.53: 34307+ [1au]
> SRV? _application._tcp.consul. (60)
> 11:06:17.587802 eth2  Out ifindex 4 aa:aa:aa:aa:aa:aa ethertype IPv4
> (0x0800), length 108: 10.10.10.50.63155 > 10.10.10.11.53: 34307+ [1au]
> SRV? _application._tcp.consul. (60)
> 11:06:17.588097 eth2  In  ifindex 4 ff:ff:ff:ff:ff:ff ethertype IPv4
> (0x0800), length 205: 10.10.10.10.53 > 10.10.10.50.24050: 2194 2/0/1 SRV
> 0a5099e5.addr.consul.:25340 1 1, SRV 0a509934.addr.consul.:26010 1 1 (157)
> 11:06:17.588097 eth2  In  ifindex 4 ff:ff:ff:ff:ff:ff ethertype IPv4
> (0x0800), length 205: 10.10.10.11.53 > 10.10.10.50.63155: 2194 2/0/1 SRV
> 0a5099e5.addr.consul.:25340 1 1, SRV 0a509934.addr.consul.:26010 1 1 (157)
> ```
>
> Both nameservers reply with the same response. But what if they are out
> of sync? Let's say one says: server1, server2 and the second one says
> server2, server3? So far testing this locally - I see sometimes the
> reply overrides the table, but sometimes it seems to just gets merged
> with the rest.
>

Most service discovery systems employ "eventually consistency".
Therefore, some instances of
the DNS servers in front of that system may have "stale" data. The
instance usually becomes
consistent with milliseconds or seconds; it depends on the topology.

I don't think haproxy performs any kind of merging, I believe it uses
the 1st valid response.


> 2) Each entry from SRV reply will be placed into the table under
> specific se_id. Most of the times that placement won't change. So, for
> the example above the most likely 0a5099e5.addr.consul. and
> 0a509934.addr.consul. will have se_id 1 and 2 respectively. However
> sometimes we have the following scenario:
>
> 1. We admistratively disable the server (drain traffic) with the next
> command:
>
> ```
> echo "set server example/application1 state maint" | nc -U
> /var/lib/haproxy/stats
> ```
>
> the MAINT flag will be added to the record with se_id 1
>
> 2. Instance of application goes down and gets de-registered from consul,
> so also evicted from srv replies and out of discovery of haproxy.
>
> 3. Instance of application goes up and gets registered by consul and
> discovered by haproxy, but haproxy allocates different se_id for it.
> Haproxy healthchecks will control the traffic to it in this case.
>
> 4. We will still have se_id 1 with MAINT flag and application instance
> dns record placed into different se_id.
>
> The problem comes that any new discovered record which get placed into
> se_id 1 will never be active until either command:
>
> ```
> echo "set server example/application1 state ready" | nc -U
> /var/lib/haproxy/stats
> ```
>
> executed or haproxy gets reloaded without state file. With this pattern
> we basically have persistent "records pollution" due to operations made
> directly with control socket.
>

This situation could lead to minor incidents where most newly
registered servers are
assigned to se_ids that were previously in maintenance mode.
So, you end up with a backend that has, let's say, 75% of servers in
maintenance mode.

> I am not sure is there anything to do about this. Maybe, if haproxy
> could cache the state not only of se_id but also associated record with
> that and then if that gets changed - re-schedule healtchecks. Or instead
> of integer ids use some hashed ids based on dns/ip-addresses of
> discovered records, in this case binding will happen exactly in the same
> slot.
>

Another approach could be haproxy resetting the `srv_admin_state `
field for IDs for which an
entry was de-registered.

My 2 cents,
Pavlos

Re: [ANNOUNCE] haproxy-3.0-dev7

[Илья Шипицин]

сб, 6 апр. 2024 г. в 17:53, Willy Tarreau :

> Hi,
>
> HAProxy 3.0-dev7 was released on 2024/04/06. It added 73 new commits
> after version 3.0-dev6.
>
> Among the changes that stand out in this version, here's what I'm seeing:
>
>   - improvements to the CLI internal API so that the various keyword
> handlers now have their own buffers. This might possibly uncover
> a few long-lasting bugs but over time will improve the reliability
> and avoid the occasional bugs with connections never closing or
> spinning loops.
>
>   - we no longer depend on libsystemd. Not only this will avoid pulling
> in tons of questionable dependencies, this also allows to enable
> USE_SYSTEMD by default (it's only done on linux-glibc though), thus
> reducing config combinations.
>
>   - log load-balancing internals were simplified. The very first version
> (never merged) didn't rely on backends, thus used to implement its
> own servers and load-balancing. It was finally remapped to backends
> and real servers, but the LB algorithms had remained specific, with
> some exceptions at various places in the setup code to handle them.
> Now the backends have switched to regular LB algorithms, which not
> only helps for code maintenance, but also exposes all table-based
> algorithms to the log backends with support for weights, and also
> exposed the "sticky" algorithm to TCP and HTTP backends. It's one of
> these changes which remove code while adding features :-)
>
>   - Linux capabilities are now properly checked so that haproxy won't
> complain about permissions for example when used in transparent mode,
> if capabilities are sufficient. In addition, file-system capabilities
> set on the binary are also supported now.
>
>   - stick-tables are now sharded over multiple tree heads each with their
> own locks. This significantly reduces locking contention on systems
> with many threads (gains of ~6x measured on a 80-thread systems). In
> addition, the locking could be reduced even with low thread counts,
> particulary when using peers, where the performance could be doubled.
>
>   - cookies are now permitted for dynamically added servers. The only
> reason they were not previously was that it required to audit the
> whole cookie initialization/release code to figure whether it had
> corner cases or not. With that audit now done, the cookies could
> be allowed. In addition, dynamic cookies were supported a bit by
> accident with a small defect (one had to set the address again to
> index the server), and are now properly supported.
>
>   - the "enabled" keyword used to be silently ignored when adding a
> dynamic server. Now it's properly rejected to avoid confusing
> scripts. We don't know yet if it will be supported later or not,
> so better stay safe.
>
>   - the key used by consistent hash to map to a server used to always
> be the server's id (either explicit or implicit, position-based).
> Now the "hash-key" directive will also allow to use the server's
> address or address+port for this. The benefit is that multiple LBs
> with servers in a different order will still send the same hashes
> to the same servers.
>
>   - a new "guid" keyword was added for servers, listeners and proxies.
> The purpose will be to make it possible for external APIs to assign
> a globally unique object identifier to each of them in stats dumps
> or CLI accesses, and to later reliably recognize a server upon
> reloads. For now the identifier is not exploited.
>

I have a question about the UUID version. it is not specified. Is it UUID
version 6 ?


>
>   - QUIC now supports the HyStart++ (RFC9406) alternative to slowstart
> with the Cubic algorithm. It's supposed to show better recovery
> patterns. More testing is needed before enabling it by default.
>
>   - a few bug fixes (truncated responses when splicing, QUIC crashes
> on strict-alignment platforms, redispatch 0 didn't work, more OCSP
> update fixes, proper reporting of too big CLI payload, etc).
>
>   - some build fixes, code cleanups, CI updates, doc updates, and
> cleanups of regtests.
>
> I think that's all. It's currently up and running on haproxy.org. I'd
> suspect that with the many stable updates yesterday, we may see less
> test reports on 3.0-dev7, but please don't forget to test it if you
> can, that helps a lot ;-)
>
> Please find the usual URLs below :
>Site index   : https://www.haproxy.org/
>Documentation: https://docs.haproxy.org/
>Wiki : https://github.com/haproxy/wiki/wiki
>Discourse: https://discourse.haproxy.org/
>Slack channel: https://slack.haproxy.org/
>Issue tracker: https://github.com/haproxy/haproxy/issues
>Sources  : https://www.haproxy.org/download/3.0/src/
>Git repository   : https://git.haproxy.org/git/haproxy.git/

Re: haproxy backend server template service discovery questions

[Илья Шипицин]

Consul template is something done by consul itself, after that haproxy.conf
is rendered

Do you mean "how haproxy deals with rendered template"?

On Fri, Apr 5, 2024, 15:02 Andrii Ustymenko 
wrote:

> Dear list!
>
> My name is Andrii. I work for Adyen. We are using haproxy as our main
> software loadbalancer at quite large scale.
> Af of now our main use-case for backends routing based on
> server-template and dynamic dns from consul as service discovery. Below
> is the example of simple backend configuration:
>
> ```
> backend example
>balance roundrobin
>server-template application 10 _application._tcp.consul resolvers
> someresolvers init-addr last,libc,none resolve-opts allow-dup-ip
> resolve-prefer ipv4 check ssl verify none
> ```
>
> and in global configuration
>
> ```
> resolvers someresolvers
>nameserver ns1 10.10.10.10:53
>nameserver ns2 10.10.10.11:53
> ```
>
> As we see haproxy will create internal table for backends with some
> be_id and be_name=application and allocate 10 records for each server
> with se_id from 1 to 10. Then those records get populated and updated
> with the data from resolvers.
> I would like to understand couple of things with regards to this
> structure and how it works, which I could not figure out myself from the
> source code:
>
> 1) In tcpdump for dns queries we see that haproxy asynchronously polls
> all the nameservers simultaneously. For instance:
>
> ```
> 11:06:17.587798 eth2  Out ifindex 4 aa:aa:aa:aa:aa:aa ethertype IPv4
> (0x0800), length 108: 10.10.10.50.24050 > 10.10.10.10.53: 34307+ [1au]
> SRV? _application._tcp.consul. (60)
> 11:06:17.587802 eth2  Out ifindex 4 aa:aa:aa:aa:aa:aa ethertype IPv4
> (0x0800), length 108: 10.10.10.50.63155 > 10.10.10.11.53: 34307+ [1au]
> SRV? _application._tcp.consul. (60)
> 11:06:17.588097 eth2  In  ifindex 4 ff:ff:ff:ff:ff:ff ethertype IPv4
> (0x0800), length 205: 10.10.10.10.53 > 10.10.10.50.24050: 2194 2/0/1 SRV
> 0a5099e5.addr.consul.:25340 1 1, SRV 0a509934.addr.consul.:26010 1 1 (157)
> 11:06:17.588097 eth2  In  ifindex 4 ff:ff:ff:ff:ff:ff ethertype IPv4
> (0x0800), length 205: 10.10.10.11.53 > 10.10.10.50.63155: 2194 2/0/1 SRV
> 0a5099e5.addr.consul.:25340 1 1, SRV 0a509934.addr.consul.:26010 1 1 (157)
> ```
>
> Both nameservers reply with the same response. But what if they are out
> of sync? Let's say one says: server1, server2 and the second one says
> server2, server3? So far testing this locally - I see sometimes the
> reply overrides the table, but sometimes it seems to just gets merged
> with the rest.
>
> 2) Each entry from SRV reply will be placed into the table under
> specific se_id. Most of the times that placement won't change. So, for
> the example above the most likely 0a5099e5.addr.consul. and
> 0a509934.addr.consul. will have se_id 1 and 2 respectively. However
> sometimes we have the following scenario:
>
> 1. We admistratively disable the server (drain traffic) with the next
> command:
>
> ```
> echo "set server example/application1 state maint" | nc -U
> /var/lib/haproxy/stats
> ```
>
> the MAINT flag will be added to the record with se_id 1
>
> 2. Instance of application goes down and gets de-registered from consul,
> so also evicted from srv replies and out of discovery of haproxy.
>
> 3. Instance of application goes up and gets registered by consul and
> discovered by haproxy, but haproxy allocates different se_id for it.
> Haproxy healthchecks will control the traffic to it in this case.
>
> 4. We will still have se_id 1 with MAINT flag and application instance
> dns record placed into different se_id.
>
> The problem comes that any new discovered record which get placed into
> se_id 1 will never be active until either command:
>
> ```
> echo "set server example/application1 state ready" | nc -U
> /var/lib/haproxy/stats
> ```
>
> executed or haproxy gets reloaded without state file. With this pattern
> we basically have persistent "records pollution" due to operations made
> directly with control socket.
>
> I am not sure is there anything to do about this. Maybe, if haproxy
> could cache the state not only of se_id but also associated record with
> that and then if that gets changed - re-schedule healtchecks. Or instead
> of integer ids use some hashed ids based on dns/ip-addresses of
> discovered records, in this case binding will happen exactly in the same
> slot.
>
> Thanks in advance!
>
> --
>
> Best regards,
>
> Andrii Ustymenko
>
>
>

Re: git clone git.haproxy.git with curl-8.7.1 failing writing received data

[Bertrand Jacquin]

On 2024-04-05 20:24, Bertrand Jacquin wrote:

Just let us know if you're interested. We can also first wait for 
Stefan
and/or Daniel's analysis of a possible cause for the commit you 
bisected

above before hacking too much stuff, though :-)


Let's see! Latest digging seems to lead to some issue with libcurl in 
this case, I might do some test to drop haproxy from the loop to 
eliminate that, but it'll take me a few hours before I can setup lab.


Well, I'm just stupid, I've a setup now bypassing HAProxy directly 
targeting nginx. I did tests against H2 with and without TLS, I'm not 
able to reproduce any of both issue (fail writing received data with 
curl 8.7.1 and git falling back to use dumb http client, and hang with 
dumb http client forced) with curl 8.7.1 with H2 enabled


Cheers,

--
Bertrand

Re: git clone git.haproxy.git with curl-8.7.1 failing writing received data

[Bertrand Jacquin]

Hey Willy,

On 2024-04-05 19:44, Willy Tarreau wrote:


Thanks a lot for these details. One thing to have in mind that could
explain that you have not observed this on other servers is that we're
using plain HTTP, we haven't deployed the git-server stuff, so maybe
it triggers a different object transfer sequence and/or code path.


You're definitely onto something here, by forcing git client to use HTTP 
dumb (GIT_SMART_HTTP=0), it's also failing against my own endpoint using 
HAProxy 2.8.7-1a82cdf, although with different symptoms, clone end up 
hanging, although (still with HTTP dumb forced) work just fine with curl 
8.6.0 with HTTP2 enabled, or curl 8.7.1 with HTTP2 disabled, so that's 
definitely an HTTP2 issue and likely not an HAProxy one. Let me know if 
you want me to drop haproxy list form recipients.


Bisecting curl again in this situation points to a different first bad 
commit: 0dc036225b30 ("HTTP/2: write response directly") 
(https://github.com/curl/curl/commit/0dc036225b3013bf994da81fa44571bcfcecbe92)


Note this issue only appears when curl is compiled with HTTP2 enabled. 
I'm
quite sure git.haproxy.org is running on bleeding edge HAProxy, which 
might
not be supporting changes brought by 463472a2d6e3, however I'm curious 
here

to understand protocol corruption.


If you want to make some tests, we can synchronize.


I've done more than enough for this week, I'm off for today, so I'm 
definitely free at the moment :)



Among the easy stuff
we can try on the haproxy side are:
  - restart with the stable version in case it has any effect
  - disable H2 (not sure if that's doable in git-clone, but we can also
easily open an extra port for you to test)


I haven't found a way to prevent git from using HTTP2, curl is small 
enough to recompile quickly to test with different features enabled. 
I've been only observing this issue with HTTP2 indeed, I'm not able to 
reproduce initial findings with curl built with HTTP2 disabled at build 
time.


Just let us know if you're interested. We can also first wait for 
Stefan
and/or Daniel's analysis of a possible cause for the commit you 
bisected

above before hacking too much stuff, though :-)


Let's see! Latest digging seems to lead to some issue with libcurl in 
this case, I might do some test to drop haproxy from the loop to 
eliminate that, but it'll take me a few hours before I can setup lab.



Cheers,
Willy


--
Bertrand

Re: git clone git.haproxy.git with curl-8.7.1 failing writing received data

[Willy Tarreau]

Hi Bertrand!

On Fri, Apr 05, 2024 at 07:27:28PM +0100, Bertrand Jacquin wrote:
> Hi,
> 
> For the last few days, I've been unable to git clone
> https://git.haproxy.org/git/haproxy.git with curl-8.7.1, where I'm getting
> the following error:
> 
>   $ GIT_TRACE=1 git fetch https://git.haproxy.org/git/haproxy.git
>   19:12:01.277740 git.c:463   trace: built-in: git fetch
> https://git.haproxy.org/git/haproxy.git
>   19:12:01.278266 run-command.c:657   trace: run_command: GIT_DIR=.git
> git remote-https https://git.haproxy.org/git/haproxy.git
> https://git.haproxy.org/git/haproxy.git
>   19:12:01.279001 git.c:749   trace: exec: git-remote-https
> https://git.haproxy.org/git/haproxy.git
> https://git.haproxy.org/git/haproxy.git
>   19:12:01.279018 run-command.c:657   trace: run_command:
> git-remote-https https://git.haproxy.org/git/haproxy.git
> https://git.haproxy.org/git/haproxy.git
>   fatal: unable to access 'https://git.haproxy.org/git/haproxy.git/': Failed
> writing received data to disk/application
(...)
> Bisecting through changes from curl 8.6.0 to 8.7.1 points me to 463472a2d6e3
> ("lib: move client writer into own source") 
> (https://github.com/curl/curl/commit/463472a2d6e3301c1468b5323b856cb67a91f579)
> as the first bad commit.

Thanks a lot for these details. One thing to have in mind that could
explain that you have not observed this on other servers is that we're
using plain HTTP, we haven't deployed the git-server stuff, so maybe
it triggers a different object transfer sequence and/or code path.

> Note this issue only appears when curl is compiled with HTTP2 enabled. I'm
> quite sure git.haproxy.org is running on bleeding edge HAProxy, which might
> not be supporting changes brought by 463472a2d6e3, however I'm curious here
> to understand protocol corruption.

If you want to make some tests, we can synchronize. Among the easy stuff
we can try on the haproxy side are:
  - restart with the stable version in case it has any effect
  - disable H2 (not sure if that's doable in git-clone, but we can also
easily open an extra port for you to test)

Just let us know if you're interested. We can also first wait for Stefan
and/or Daniel's analysis of a possible cause for the commit you bisected
above before hacking too much stuff, though :-)

Cheers,
Willy

Re: [PATCH] DOC: configuration: grammar fixes for strict-sni

[Willy Tarreau]

Hi Nicolas,

On Wed, Apr 03, 2024 at 01:52:22PM +0200, Nicolas CARPi wrote:
> Hello,
> 
> Please find attached a little patch for the "strict-sni" configuration 
> documentation, which had incorrect grammar.

Now merged, thank you!
Willy

Re: [PATCH] MINOR: systemd: Include MONOTONIC_USEC field in RELOADING=1 message

[Lukas Tribus]

On Thu, 4 Apr 2024 at 16:00, Tim Düsterhus  wrote:
>
> Hi
>
> On 4/4/24 14:35, William Lallemand wrote:
> > I'm not against merging this, but I don't see any change comparing to the
> > current model?
> >
>
> I mainly stumbled upon this new mode in the documentation while looking
> into replacing libsystemd, where you beat me to it :-)

Ah, that's nice, it's already gone in commit aa3632962 ("MEDIUM:
mworker: get rid of libsystemd") - I was thinking about the very same
thing ;)


Lukas

Re: [PATCH] MINOR: systemd: Include MONOTONIC_USEC field in RELOADING=1 message

[William Lallemand]

On Thu, Apr 04, 2024 at 04:00:16PM +0200, Tim Düsterhus wrote:
> Hi
> 
> On 4/4/24 14:35, William Lallemand wrote:
> > I'm not against merging this, but I don't see any change comparing to the
> > current model?
> > 
> 
> I mainly stumbled upon this new mode in the documentation while looking into
> replacing libsystemd, where you beat me to it :-)
> 
> My understanding is that it improves the situation insofar that systemd
> knows that the reload (attempt) is not yet finished even if the kill command
> exits.
> 
> Currently it's the following:
> 
> - systemd executes the kill.
> - kill exists.
> - systemd believes the reload finished.
> - HAProxy sees the signal, sends RELOADING=1.
> - systemd believes a second reload started.
> - HAProxy finishes reloading, sends READY=1.
> - systemd believes the second reload finished.
> With the new mode it's:
> 
> - systemd sends the signal.
> - systemd waits for RELOADING=1.
> - HAProxy sees the signal, sends RELOADING=1.
> - HAProxy finishes reloading, sends READY=1.
> - systemd believes the reload finished.
> 
> The new mode is only available with very recent systemd versions, but we can
> future-proof the implementation by already including the MONOTONIC_USEC
> field in the message. Unknown fields are explicitly ignored by systemd, thus
> this patch makes the situation no worse.

I thought systemd was already waiting the READY=1 message in fact, I
made a few tests which confirms your explanation, thanks, merged!

What I find stupid is that it's not possible to have the red
"systemd[1]: Reload failed for haproxy.service" message when using all
these reload method...

For example if you replace `kill`by `false` in the unit file, you will
have:

wla@kikyo:~% systemctl status haproxy
● haproxy.service - HAProxy Load Balancer
 Loaded: loaded (/etc/systemd/system/haproxy.service; enabled; 
preset: enabled)
 Active: active (running) since Thu 2024-04-04 15:04:20 CEST; 1h 
4min ago
   Docs: man:haproxy(1)
 file:/usr/share/doc/haproxy/configuration.txt.gz
Process: 58596 ExecReload=/usr/bin/false (code=exited, 
status=1/FAILURE)
   Main PID: 57180 (haproxy)
 Status: "Ready."
  Tasks: 9 (limit: 18702)
 Memory: 74.6M
CPU: 1.624s
 CGroup: /system.slice/haproxy.service
 ├─57180 /usr/sbin/haproxy -Ws -f /etc/haproxy/haproxy.cfg 
-p /run/haproxy.pid -S /run/haproxy-master.sock
 └─57601 /usr/sbin/haproxy -sf 57557 -x sockpair@5 -Ws -f 
/etc/haproxy/haproxy.cfg -p /run/haproxy.pid -S /run/haproxy-master.sock

Apr 04 15:14:12 kikyo systemd[1]: Reload failed for haproxy.service - 
HAProxy Load Balancer.
Apr 04 15:14:33 kikyo systemd[1]: Reloading haproxy.service - HAProxy 
Load Balancer...
Apr 04 15:14:33 kikyo systemd[1]: haproxy.service: Control process 
exited, code=exited, status=1/FAILURE
Apr 04 15:14:33 kikyo systemd[1]: Reload failed for haproxy.service - 
HAProxy Load Balancer.
Apr 04 15:14:48 kikyo systemd[1]: Reloading haproxy.service - HAProxy 
Load Balancer...
Apr 04 15:14:48 kikyo systemd[1]: haproxy.service: Control process 
exited, code=exited, status=1/FAILURE
Apr 04 15:14:48 kikyo systemd[1]: Reload failed for haproxy.service - 
HAProxy Load Balancer.
Apr 04 15:17:20 kikyo systemd[1]: Reloading haproxy.service - HAProxy 
Load Balancer...
Apr 04 15:17:20 kikyo systemd[1]: haproxy.service: Control process 
exited, code=exited, status=1/FAILURE
Apr 04 15:17:20 kikyo systemd[1]: Reload failed for haproxy.service - 
HAProxy Load Balancer.

With messages hilighted in red, and a clear failure of the reload on the
"Process" line.

I can't find a way of achieving the same thing with sd_notify() :(

-- 
William Lallemand

Re: [PATCH] MINOR: systemd: Include MONOTONIC_USEC field in RELOADING=1 message

[Tim Düsterhus]

Hi

On 4/4/24 14:35, William Lallemand wrote:

I'm not against merging this, but I don't see any change comparing to the
current model?



I mainly stumbled upon this new mode in the documentation while looking 
into replacing libsystemd, where you beat me to it :-)


My understanding is that it improves the situation insofar that systemd 
knows that the reload (attempt) is not yet finished even if the kill 
command exits.


Currently it's the following:

- systemd executes the kill.
- kill exists.
- systemd believes the reload finished.
- HAProxy sees the signal, sends RELOADING=1.
- systemd believes a second reload started.
- HAProxy finishes reloading, sends READY=1.
- systemd believes the second reload finished.

With the new mode it's:

- systemd sends the signal.
- systemd waits for RELOADING=1.
- HAProxy sees the signal, sends RELOADING=1.
- HAProxy finishes reloading, sends READY=1.
- systemd believes the reload finished.

The new mode is only available with very recent systemd versions, but we 
can future-proof the implementation by already including the 
MONOTONIC_USEC field in the message. Unknown fields are explicitly 
ignored by systemd, thus this patch makes the situation no worse.


Best regards
Tim Düsterhus

Re: [PATCH] MINOR: systemd: Include MONOTONIC_USEC field in RELOADING=1 message

[William Lallemand]

On Wed, Apr 03, 2024 at 10:39:16PM +0200, Tim Duesterhus wrote:
> As per the `sd_notify` manual:
> 
> > A field carrying the monotonic timestamp (as per CLOCK_MONOTONIC) formatted
> > in decimal in μs, when the notification message was generated by the client.
> > This is typically used in combination with "RELOADING=1", to allow the
> > service manager to properly synchronize reload cycles. See 
> > systemd.service(5)
> > for details, specifically "Type=notify-reload".
> 
> Thus this change allows users with a recent systemd to switch to
> `Type=notify-reload`, should they desire to do so. Correct behavior was
> verified with a Fedora 39 VM.
> 
> see systemd/systemd#25916
> ---
>  src/haproxy.c | 13 +++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/src/haproxy.c b/src/haproxy.c
> index 0fcc3e5416..a5f1e79ef9 100644
> --- a/src/haproxy.c
> +++ b/src/haproxy.c
> @@ -844,8 +844,17 @@ void mworker_reload(int hardreload)
>   }
>  
>  #if defined(USE_SYSTEMD)
> - if (global.tune.options & GTUNE_USE_SYSTEMD)
> - sd_notify(0, "RELOADING=1\nSTATUS=Reloading Configuration.\n");
> + if (global.tune.options & GTUNE_USE_SYSTEMD) {
> + struct timespec ts;
> +
> + (void)clock_gettime(CLOCK_MONOTONIC, );
> +
> + sd_notifyf(0,
> +"RELOADING=1\n"
> +"STATUS=Reloading Configuration.\n"
> +"MONOTONIC_USEC=%" PRIu64 "\n",
> +(ts.tv_sec * 100ULL + ts.tv_nsec / 1000ULL));
> + }
>  #endif
>   mworker_reexec(hardreload);
>  }

It looks like they still failed to implement a correct reload status
with this mode. Honestly I don't get it, to me it does not improve
anything :/

Given that the only signal we can send is READY=1 and that only a
failure of a ExecReload= command would set a reloading error, it does
exactly the same as the current kill -USR2 method.

I think only implementing a synchronous `haproxyctl reload` command
which uses the master CLI could improve the situation, only that could
return a failure and emits the error output...

I'm not against merging this, but I don't see any change comparing to the
current model?

-- 
William Lallemand

Re: [PATCH 0/1] CI: extend Fedora Rawhide to run x86 bit as well

[Willy Tarreau]

On Wed, Apr 03, 2024 at 08:56:21PM +0200, Ilya Shipitsin wrote:
> it seems to be the easiest to build "m32" on Fedora comparing to Ubuntu, let's
> stick on that for a while

OK, now merged, we'll see. Thank you!
Willy

Re: How to check if a domain is known to HAProxy

[Shawn Heisey]

On 4/3/24 06:02, Froehlich, Dominik wrote:
I fear that strict-sni won’t get us far. The issue is that the SNI is 
just fine (it is in the crt-list), however we also need to check if the 
host-header is part of the crt-list. E.g.


William's answer should work.

The strict-sni setting makes sure that the SNI is in the cert list.  If 
it's not, then TLS negotiation will fail and as a result the request 
will not complete.


Then the following ACL in William's reply checks that the host header 
actually matches SNI:


   http-request set-var(txn.host) hdr(host)
   # Check whether the client is attempting domain fronting.
   acl ssl_sni_http_host_match ssl_fc_sni,strcmp(txn.host) eq 0

If SNI matches the Host header, then that ACL will be true.  Combined 
with strict-sni ensuring that the SNI matches one of your certs, this 
will get you what you want.


You can also reverse the ACL so it is false if there is no match.  The 
docs for 2.8 do not mention "ne" as a possible operator, so this ACL 
checks for greater than and less than:


   acl ssl_sni_http_host_no_match ssl_fc_sni,strcmp(txn.host) lt 0
   acl ssl_sni_http_host_no_match ssl_fc_sni,strcmp(txn.host) gt 0

Thanks,
Shawn

Re: How to check if a domain is known to HAProxy

[Froehlich, Dominik]

Hello Willian,

Thank you for your response.

I fear that strict-sni won’t get us far. The issue is that the SNI is just fine 
(it is in the crt-list), however we also need to check if the host-header is 
part of the crt-list. E.g.

curl https://my-host.domain.com<https://my-host.domain.com/> -H “host: 
other-host.otherdomain.com”

so here we check for the SNI “my-host.domain.com” automatically via crt-list.

but in the next step we select the backend based on the host-header, but only 
if it also is present in the crt-list (which we use as a list of valid domains 
hosted on the platform)

so based on what you said we can’t do that, we would do something like

http-request set-var(txn.forwarded_host) req.hdr(host),host_only,lower

acl is_known_domain var(txn.forwarded_host),map_dom(/domains.map) -m found

http request-deny if ! is_known_domain

where /domains.map is basically a copy of the crt-list like that:

*.domain.com 1
*.otherdomain.com 1

So, this works, though it is ugly because I need to do double-maintenance of 
the crt-list.
Even if I used strict-sni, you could still run into the issue that SNI is on 
the crt-list, but the host-header is not.



From: William Lallemand 
Date: Wednesday, 3. April 2024 at 11:31
To: Froehlich, Dominik 
Cc: haproxy@formilux.org 
Subject: Re: How to check if a domain is known to HAProxy
On Wed, Apr 03, 2024 at 07:47:44AM +, Froehlich, Dominik wrote:
> Subject: How to check if a domain is known to HAProxy
> Hello everyone,
>
> This may be kind of a peculiar request.
>
> We have the need to block requests that are not in the crt-list of our 
> frontend.
>
> So, the expectation would be that HAProxy does a lookup of the domain (as it 
> does for the crt-list entry) but for domain-fronted requests, i.e. we have to 
> check both the SNI and the host header.
>
> What makes it difficult is that we still want to allow domain-fronting, but 
> only if the host header also matches an entry in the crt-list.
>
> At the moment, I don’t see any way of doing this programmatically, and the 
> crt-list lookup based on the SNI is completely within HAProxy logic.
>
> Is there any way to access the crt-list via an ACL or similar? The 
> alternative would be to maintain the list twice and add it as a map or list 
> to the HAProxy config and then maybe do a custom host matching via LUA script 
> etc. but I really would like to avoid that.
>
> Any hints from the community?
>

Hello,

You can't access the crt-list from the ACL, however if you are using the
`strict-sni` keyword, you will be sure that the requested SNI will be in
your crt-list. And then you can compare the host header with the SNI.

There is an example in the strcmp keyword documentation:

   http-request set-var(txn.host) hdr(host)
   # Check whether the client is attempting domain fronting.
   acl ssl_sni_http_host_match ssl_fc_sni,strcmp(txn.host) eq 0


https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.haproxy.org%2F2.9%2Fconfiguration.html%23strcmp=05%7C02%7Cdominik.froehlich%40sap.com%7Cef9d69783ff54043a83708dc53c0deae%7C42f7676cf455423c82f6dc2d99791af7%7C0%7C0%7C638477335041142353%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C=d8jyQKbe7ODqCI%2BCklprFW9LC67b5yXwHHJYJEQhRGk%3D=0<https://docs.haproxy.org/2.9/configuration.html#strcmp>

Regards,

--
William Lallemand

Re: How to check if a domain is known to HAProxy

[William Lallemand]

On Wed, Apr 03, 2024 at 07:47:44AM +, Froehlich, Dominik wrote:
> Subject: How to check if a domain is known to HAProxy
> Hello everyone,
> 
> This may be kind of a peculiar request.
> 
> We have the need to block requests that are not in the crt-list of our 
> frontend.
> 
> So, the expectation would be that HAProxy does a lookup of the domain (as it 
> does for the crt-list entry) but for domain-fronted requests, i.e. we have to 
> check both the SNI and the host header.
> 
> What makes it difficult is that we still want to allow domain-fronting, but 
> only if the host header also matches an entry in the crt-list.
> 
> At the moment, I don’t see any way of doing this programmatically, and the 
> crt-list lookup based on the SNI is completely within HAProxy logic.
> 
> Is there any way to access the crt-list via an ACL or similar? The 
> alternative would be to maintain the list twice and add it as a map or list 
> to the HAProxy config and then maybe do a custom host matching via LUA script 
> etc. but I really would like to avoid that.
> 
> Any hints from the community?
> 

Hello,

You can't access the crt-list from the ACL, however if you are using the
`strict-sni` keyword, you will be sure that the requested SNI will be in
your crt-list. And then you can compare the host header with the SNI.

There is an example in the strcmp keyword documentation:

   http-request set-var(txn.host) hdr(host)
   # Check whether the client is attempting domain fronting.
   acl ssl_sni_http_host_match ssl_fc_sni,strcmp(txn.host) eq 0


https://docs.haproxy.org/2.9/configuration.html#strcmp

Regards,

-- 
William Lallemand

Re: Error While deviceatlas 3.2.2 and haproxy 2.9.6 make from source

[Willy Tarreau]

On Wed, Apr 03, 2024 at 06:21:22AM +0100, David CARLIER wrote:
> Hi all,
> 
> Thanks for your report. This is a known issue the 3.2.3 release is
> scheduled within this month.

Even better :-)

Thanks David!
Willy

Re: Error While deviceatlas 3.2.2 and haproxy 2.9.6 make from source

[David CARLIER]

Hi all,

Thanks for your report. This is a known issue the 3.2.3 release is
scheduled within this month.

Regards.

On Wed, 3 Apr 2024 at 04:38, Willy Tarreau  wrote:

> Hello,
>
> On Wed, Apr 03, 2024 at 05:21:03AM +0530, Mahendra Patil wrote:
> > /opt/deviceatlas/Src//dac.c: In function ātoverdecā:
> > /opt/deviceatlas/Src//dac.c:714:13: warning: implicit declaration of
> > function ā__builtin_sadd_overflowā [-Wimplicit-function-declaration]
> >  if (DATLAS_A_OFLOW(cur * 10, decrm, )) {
> (...)
> > /opt/deviceatlas/Src//dac.o: In function `toverdec':
> > /opt/deviceatlas/Src//dac.c:714: undefined reference to
> > `__builtin_sadd_overflow'
> > collect2: error: ld returned 1 exit status
> > make: *** [haproxy] Error 1
>
> From what I'm seeing, __builtin_sadd_overflow() first appeared in gcc-5,
> so you don't have it on your system, which seems to be RHEL 7 or CentOS 7
> based on the compiler version (gcc 4.8.5).
>
> I don't know how important is the use of this builtin for Device Atlas,
> I'll let David check. As a hack you could verify that it builds when you
> change it to:
>
> if ((r = cur*10 + decrm), 0) {
>
> But be careful that removing this overflow check might introduce a
> vulnerability, so if this builds, please do not deploy such code without
> David's approval.
>
> Another approach could be to build gcc-5.5 on your distro. It's not that
> hard but might not be what you were expecting to do. There are various
> howtos on the net, such as here:
>
>   https://gist.github.com/tyleransom/2c96f53a828831567218eeb7edc2b1e7
>
> Though this one will replace the default compiler in your path, and you
> may likely want to add "--program-suffix=-5.5" to the configure (and
> replace 5.4 with 5.5 everywhere) so that you can then pass "CC=gcc-5.5"
> to haproxy's "make" command line.
>
> Hoping this helps,
> Willy
>

Re: Error While deviceatlas 3.2.2 and haproxy 2.9.6 make from source

[Willy Tarreau]

Hello,

On Wed, Apr 03, 2024 at 05:21:03AM +0530, Mahendra Patil wrote:
> /opt/deviceatlas/Src//dac.c: In function âtoverdecâ:
> /opt/deviceatlas/Src//dac.c:714:13: warning: implicit declaration of
> function â__builtin_sadd_overflowâ [-Wimplicit-function-declaration]
>  if (DATLAS_A_OFLOW(cur * 10, decrm, )) {
(...)
> /opt/deviceatlas/Src//dac.o: In function `toverdec':
> /opt/deviceatlas/Src//dac.c:714: undefined reference to
> `__builtin_sadd_overflow'
> collect2: error: ld returned 1 exit status
> make: *** [haproxy] Error 1

>From what I'm seeing, __builtin_sadd_overflow() first appeared in gcc-5,
so you don't have it on your system, which seems to be RHEL 7 or CentOS 7
based on the compiler version (gcc 4.8.5).

I don't know how important is the use of this builtin for Device Atlas,
I'll let David check. As a hack you could verify that it builds when you
change it to:

if ((r = cur*10 + decrm), 0) {

But be careful that removing this overflow check might introduce a
vulnerability, so if this builds, please do not deploy such code without
David's approval.

Another approach could be to build gcc-5.5 on your distro. It's not that
hard but might not be what you were expecting to do. There are various
howtos on the net, such as here:

  https://gist.github.com/tyleransom/2c96f53a828831567218eeb7edc2b1e7

Though this one will replace the default compiler in your path, and you
may likely want to add "--program-suffix=-5.5" to the configure (and
replace 5.4 with 5.5 everywhere) so that you can then pass "CC=gcc-5.5"
to haproxy's "make" command line.

Hoping this helps,
Willy

Re: Dataplane exits at haproxytech/haproxy-ubuntu:2.9 in Containers

[Aleksandar Lazic]

Hi.

On 2024-03-18 (Mo.) 12:19, William Lallemand wrote:

On Sun, Mar 17, 2024 at 07:53:17PM +0100, Aleksandar Lazic wrote:

Hi.

Looks like there was a similar question in the forum
https://discourse.haproxy.org/t/trouble-with-starting-the-data-plane-api/9200

Any idea how to fix this?



Honestly no idea, you should try an issue there: 
https://github.com/haproxytech/dataplaneapi/issues


Thank you for the hint.
I have created this issue https://github.com/haproxytech/dataplaneapi/issues/329

Regards
Alex

Re: [PATCH 0/1] CI improvement, display coredumps if any

[Willy Tarreau]

On Wed, Mar 27, 2024 at 04:49:53PM +0100, Ilya Shipitsin wrote:
> it is pretty rare case, however displaying "bt" may provide some ideas what 
> went wrong

Applied, thanks Ilya! I think this will sometimes be quite helpful
because till now it was only "grrr... sig11 and we don't know why".

Willy

Re: [PATCH 1/2] REGTESTS: Do not use REQUIRE_VERSION for HAProxy 2.5+ (4)

[Willy Tarreau]

Hi Tim!

On Fri, Mar 29, 2024 at 05:12:47PM +0100, Tim Duesterhus wrote:
> Introduced in:
> 
> dfb1cea69 REGTESTS: promex: Adapt script to be less verbose
> 36d936dd1 REGTESTS: write a full reverse regtest
> b57f15158 REGTESTS: provide a reverse-server test with name argument
> f0bff2947 REGTESTS: provide a reverse-server test
> 
> see also:
> 
> fbbbc33df REGTESTS: Do not use REQUIRE_VERSION for HAProxy 2.5+
(...)

Both series applied (regtests and cocci).
Thank you!
Willy

Re: [PATCH] MEDIUM: lb-chash: Deterministic node hashes based on server address

[Willy Tarreau]

Hi Anthony,

On Mon, Apr 01, 2024 at 11:47:54AM -0400, Anthony Deschamps wrote:
> Hi Willy,
> 
> Those changes are easy enough to make, so I've attached the patch again
> with those changes. I had to make a few small adjustments to the commit
> message anyway (some things that changed as a result of reviewing this
> path). Let me know if there's anything else that I can help with.

Looks perfect now, I've merged it. Thank you very much!
Willy

Re: [PATCH] MEDIUM: lb-chash: Deterministic node hashes based on server address

[Anthony Deschamps]

Hi Willy,

Those changes are easy enough to make, so I've attached the patch again
with those changes. I had to make a few small adjustments to the commit
message anyway (some things that changed as a result of reviewing this
path). Let me know if there's anything else that I can help with.

Thank you,
Anthony
From 30896b2f6368cd022bfb574d4d5f647483b79a59 Mon Sep 17 00:00:00 2001
From: Anthony Deschamps 
Date: Fri, 16 Feb 2024 16:00:35 -0500
Subject: [PATCH] MEDIUM: lb-chash: Deterministic node hashes based on server
 address

Motivation: When services are discovered through DNS resolution, the order in
which DNS records get resolved and assigned to servers is arbitrary. Therefore,
even though two HAProxy instances using chash balancing might agree that a
particular request should go to server3, it is likely the case that they have
assigned different IPs and ports to the server in that slot.

This patch adds a server option, "hash-key " which can be set to "id" (the
existing behaviour, default), "addr", or "addr-port". By deriving the keys for
the chash tree nodes from a server's address and port we ensure that independent
HAProxy instances will agree on routing decisions. If an address is not known
then the key is derived from the server's puid as it was previously.

When adjusting a server's weight, we now check whether the server's hash has
changed. If it has, we have to remove all its nodes first, since the node keys
will also have to change.
---
 doc/configuration.txt  | 25 +++
 include/haproxy/server-t.h |  9 
 src/lb_chash.c | 91 +++---
 src/server.c   | 31 +
 4 files changed, 150 insertions(+), 6 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 1065e6098..0d5649d34 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -4813,6 +4813,8 @@ error-log-format  X  X X -
 force-persist -  - X X
 filter-  X X X
 fullconn  X  - X X
+hash-balance-factor   X  - X X
+hash-key  X  - X X
 hash-type X  - X X
 http-after-response   X (!)  X X X
 http-check commentX  - X X
@@ -6606,6 +6608,29 @@ hash-balance-factor 
   See also : "balance" and "hash-type".
 
 
+hash-key 
+  Specify how "hash-type consistent" node keys are computed
+
+  Arguments :
+may be one of the following :
+
+  id The node keys will be derived from the server's numeric
+ identifier as set from "id" or which defaults to its position
+ in the server list.
+
+  addr   The node keys will be derived from the server's address, when
+ available, or else fall back on "id".
+
+  addr-port  The node keys will be derived from the server's address and
+ port, when available, or else fall back on "id".
+
+  The "addr" and "addr-port" options may be useful in scenarios where multiple
+  HAProxy processes are balancing traffic to the same set of servers. If the
+  server order of each process is different (because, for example, DNS records
+  were resolved in different orders) then this will allow each independent
+  HAProxy processes to agree on routing decisions.
+
+
 hash-type   
   Specify a method to use for mapping hashes to servers
 
diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index f077ff2f8..811eae712 100644
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -223,6 +223,13 @@ struct pid_list {
 	int exited;
 };
 
+/* srv methods of computing chash keys */
+enum srv_hash_key {
+	SRV_HASH_KEY_ID = 0, /* derived from server puid */
+	SRV_HASH_KEY_ADDR,   /* derived from server address */
+	SRV_HASH_KEY_ADDR_PORT   /* derived from server address and port */
+};
+
 /* A tree occurrence is a descriptor of a place in a tree, with a pointer back
  * to the server itself.
  */
@@ -366,6 +373,8 @@ struct server {
 	struct tree_occ *lb_nodes;  /* lb_nodes_tot * struct tree_occ */
 	unsigned lb_nodes_tot;  /* number of allocated lb_nodes (C-HASH) */
 	unsigned lb_nodes_now;  /* number of lb_nodes placed in the tree (C-HASH) */
+	enum srv_hash_key hash_key; /* method to compute node hash (C-HASH) */
+	unsigned lb_server_key; /* hash of the values indicated by "hash_key" (C-HASH) */
 
 	const struct netns_entry *netns;/* contains network namespace name or NULL. Network namespace comes from configuration */
 	struct xprt_ops *xprt;  /* 

Re: Help tracking "connection refused" under pressure on v2.9

[Ricardo Nabinger Sanchez]

Hi Willy,

On Fri, 29 Mar 2024 07:17:56 +0100
Willy Tarreau  wrote:

> > These "connection refused" is from our watchdog; but the effects are as
> > perceptible from the outside.  When our watchdog hits this situation,
> > it will forcefully restart HAProxy (we have 2 instances) because there
> > will be a considerable service degradation.  If you remember, there's
> > https://github.com/haproxy/haproxy/issues/1895 and we talked briefly
> > about this in person, at HAProxyConf.  
> 
> Thanks for the context. I didn't remember about the issue. I remembered
> we discussed for a while but didn't remember about the issue in question
> obviously, given the number of issues I'm dealing with :-/
> 
> In the issue above I'm seeing an element from Felipe saying that telnet
> to port 80 can take between 3 seconds to accept. That really makes me
> think about either the SYN queue being full, causing drops and retransmits,
> or a lack of socket memory to accept packets. That one could possibly be
> caused by tcp_mem not being large enough due to some transfers with high
> latency fast clients taking a lot of RAM, but it should not affect the
> local UNIX socket. Also, killing the process means killing all the
> associated connections and will definitely result in freeing a huge
> amount of network buffers, so it could fuel that direction. If you have
> two instances, did you notice if the two start to behave badly at the
> same time ? If that's the case, it would definitely indicate a possible
> resource-based cause like socket memory etc.

Of our 2 HAProxy instances, it is usually one (mostly the frontend one)
that exhibits this behavior.  And as it is imperative that the
corrective action be as swift as possible, all instances are terminated
(which can include older instances, from graceful reloads), and new
instances are started.  Very harsh, but at >50 Gbps, each full second
of downtime adds up considerably to network pressure.

So for context, our least capable machine has 256 GB RAM.  We have not
seen any spikes over the metrics we monitor, and this issue tends to
happen at a very stable steady-state, albeit a loaded one.  While it is
currently outside of our range for detailed data, we didn't notice
anything unusual, especially regarding memory usage, on these traps we
reported.

But of course, there could be a metric that we're not yet aware that
correlates.  Anyone from the dustier, darkest corners that you know of?
:-)


> 
> > But this is incredibly elusive to reproduce; it comes and goes.  It
> > might happen every few minutes, or not happen at all for months.  Not
> > tied to a specific setup: different versions, kernels, machines.  In
> > fact, we do not have better ways to detect the situation, at least not
> > as fast, reactive, and resilient.  
> 
> It might be useful to take periodic snapshots of /proc/slabinfo and
> see if something jumps during such incidents (grep for TCP, net, skbuff
> there). I guess you have not noticed any "out of socket memory" nor such
> indications in your kernel logs, of course :-/

We have no indications of memory pressure related to network.  At the
peak, we usually see something like 15~22% overall active memory (it
fails me, but it might take >70% of active memory for these machines to
actually degrade, maybe more).  As for TCP stuff, around 16~30k active
sockets, plus some 50~100k in timewait, and still not creating any
problems.


> 
> Another one that could make sense to monitor is "PoolFailed" in
> "show info". It should always remain zero.

We collect this (all available actually); I don't remember this one
ever measuring more than zero.  But we'll keep an eye on it.

In time, could this be somewhat unrelated to HAProxy?  I.e., maybe
kernel?

Cheers,

-- 
Ricardo Nabinger Sanchez https://www.taghos.com.br/

Re: Help tracking "connection refused" under pressure on v2.9

[Willy Tarreau]

Hi Ricardo,

On Thu, Mar 28, 2024 at 06:21:16PM -0300, Ricardo Nabinger Sanchez wrote:
> Hi Willy,
> 
> On Thu, 28 Mar 2024 04:37:11 +0100
> Willy Tarreau  wrote:
> 
> > Thanks guys! So there seems to be an annoying bug. However I'm not sure
> > how this is related to your "connection refused", except if you try to
> > connect at the moment the process crashes and restarts, of course. I'm
> > seeing that the bug here is stktable_requeue_exp() calling task_queue()
> > with an invalid task expiration. I'm having a look now. I'll respond in
> > the issue with what I can find, thanks for your report.
> 
> These "connection refused" is from our watchdog; but the effects are as
> perceptible from the outside.  When our watchdog hits this situation,
> it will forcefully restart HAProxy (we have 2 instances) because there
> will be a considerable service degradation.  If you remember, there's
> https://github.com/haproxy/haproxy/issues/1895 and we talked briefly
> about this in person, at HAProxyConf.

Thanks for the context. I didn't remember about the issue. I remembered
we discussed for a while but didn't remember about the issue in question
obviously, given the number of issues I'm dealing with :-/

In the issue above I'm seeing an element from Felipe saying that telnet
to port 80 can take between 3 seconds to accept. That really makes me
think about either the SYN queue being full, causing drops and retransmits,
or a lack of socket memory to accept packets. That one could possibly be
caused by tcp_mem not being large enough due to some transfers with high
latency fast clients taking a lot of RAM, but it should not affect the
local UNIX socket. Also, killing the process means killing all the
associated connections and will definitely result in freeing a huge
amount of network buffers, so it could fuel that direction. If you have
two instances, did you notice if the two start to behave badly at the
same time ? If that's the case, it would definitely indicate a possible
resource-based cause like socket memory etc.

> But this is incredibly elusive to reproduce; it comes and goes.  It
> might happen every few minutes, or not happen at all for months.  Not
> tied to a specific setup: different versions, kernels, machines.  In
> fact, we do not have better ways to detect the situation, at least not
> as fast, reactive, and resilient.

It might be useful to take periodic snapshots of /proc/slabinfo and
see if something jumps during such incidents (grep for TCP, net, skbuff
there). I guess you have not noticed any "out of socket memory" nor such
indications in your kernel logs, of course :-/

Another one that could make sense to monitor is "PoolFailed" in
"show info". It should always remain zero.

> > Since you were speaking about FD count and maxconn at 900k, please let
> > me take this opportunity for a few extra sanity checks. By default we
> > assign up to about 50% of the FD to pipes (i.e. up to 25% pipes compared
> > to connections), so if maxconn is 900k you can reach 1800 + 900 = 2700k
> > FD. One thing to keep in mind is that /proc/sys/fs/nr_open sets a
> > per-process hard limit and usually is set to 1M, and that
> > /proc/sys/fs/file-max sets a system-wide limit and depends on the amount
> > of RAM, so both may interact with such a large setting. We could for
> > example imagine that at ~256k connections with as many pipes you're
> > reaching around 1M FDs and that the connection from socat to the CLI
> > socket cannot be accepted and is rejected. Since you recently updated
> > your kernel, it might be worth checking if the default values are still
> > in line with your usage.
> 
> We set our defaults pretty high in anticipation:
> 
>   /proc/sys/fs/file-max = 5M;
>   /proc/sys/fs/nr_open = 3M;

OK!

> Even with our software stack, we do not reach the limits.  A long time
> ago we did hit (lower limits back then) and the effects are devastating.

Yes, that's always the problem with certain limits, they hit you at the
worst ever moments, when the most users are counting on you to work fine
and when it's the hardest to spot anomalies.

Willy

Re: RFC: PKCS#11 create private keys in worker process

[Richard Chan]

"Did you identify why the fork was causing an issue? We should probably
try to understand this before, it could be something stupid in haproxy's
code or in the pkcs11 provider."

- PKCS#11 drivers contain session objects and handles to private keys in
the HSM; these session objects and handles don't always behave
well after fork - this is a known problem of a lot of existing PKCS#11
drivers
Indeed GnuTLS says:

"Note that, PKCS #11 modules behave in a peculiar way after a fork; they
require a
reinitialization of all the used PKCS #11 resources. While GnuTLS automates
that process,
there are corner cases where it is not possible to handle it correctly in
an automated way.
For that, it is recommended not to mix fork() and PKCS #11 module usage.
It is recommended to initialize and use any PKCS #11 resources in a single
process"

This is not a problem of HAProxy specifically

- having said this new PKCS#11 v3 drivers may support "fork tolerant
semantics": such
drivers don't need this kind of RFC which is targeted to drivers without
this feature.
https://docs.oasis-open.org/pkcs11/pkcs11-base/v3.0/cs01/pkcs11-base-v3.0-cs01.html

- the double caching(ssl_ckch.c/ssl_sock.c) is inelegant and messy just to
be able
to recall the PEM bytes. I think I have found a way to do it without cache.
-- cache PEM data by extending (struct ckch_data*)
-- store the (struct ckch_data*) in the SSL_CTX* using SSL_CTX_set_app_data
-- let me think about this and come up with another RFC.

Cheers
Richard

Re: Help tracking "connection refused" under pressure on v2.9

[Ricardo Nabinger Sanchez]

Hi Willy,

On Thu, 28 Mar 2024 04:37:11 +0100
Willy Tarreau  wrote:

> Thanks guys! So there seems to be an annoying bug. However I'm not sure
> how this is related to your "connection refused", except if you try to
> connect at the moment the process crashes and restarts, of course. I'm
> seeing that the bug here is stktable_requeue_exp() calling task_queue()
> with an invalid task expiration. I'm having a look now. I'll respond in
> the issue with what I can find, thanks for your report.

These "connection refused" is from our watchdog; but the effects are as
perceptible from the outside.  When our watchdog hits this situation,
it will forcefully restart HAProxy (we have 2 instances) because there
will be a considerable service degradation.  If you remember, there's
https://github.com/haproxy/haproxy/issues/1895 and we talked briefly
about this in person, at HAProxyConf.

But this is incredibly elusive to reproduce; it comes and goes.  It
might happen every few minutes, or not happen at all for months.  Not
tied to a specific setup: different versions, kernels, machines.  In
fact, we do not have better ways to detect the situation, at least not
as fast, reactive, and resilient.


> 
> Since you were speaking about FD count and maxconn at 900k, please let
> me take this opportunity for a few extra sanity checks. By default we
> assign up to about 50% of the FD to pipes (i.e. up to 25% pipes compared
> to connections), so if maxconn is 900k you can reach 1800 + 900 = 2700k
> FD. One thing to keep in mind is that /proc/sys/fs/nr_open sets a
> per-process hard limit and usually is set to 1M, and that
> /proc/sys/fs/file-max sets a system-wide limit and depends on the amount
> of RAM, so both may interact with such a large setting. We could for
> example imagine that at ~256k connections with as many pipes you're
> reaching around 1M FDs and that the connection from socat to the CLI
> socket cannot be accepted and is rejected. Since you recently updated
> your kernel, it might be worth checking if the default values are still
> in line with your usage.

We set our defaults pretty high in anticipation:

/proc/sys/fs/file-max = 5M;
/proc/sys/fs/nr_open = 3M;

Even with our software stack, we do not reach the limits.  A long time
ago we did hit (lower limits back then) and the effects are devastating.

Cheers,

-- 
Ricardo Nabinger Sanchez https://www.taghos.com.br/

Re: RFC: PKCS#11 create private keys in worker process

[William Lallemand]

On Thu, Mar 28, 2024 at 08:26:58AM +0800, Richard Chan wrote:
> Hello,
> 
> This is an RFC to recreate private keys in the worker process
> for PKCS#11, so that HSM keys can be used in -W mode.
> 
> - ssl_ckch.c: add map of ckch_data to PEM data
> - ssl_sock.c: add map of SSL_CTX* to ckch_data
> - maps are implemented using buckets of linked lists
>   it is explicit and in the code for easier review instead of using
>   more optimized hashmap implementations
> - when the SSL context is created and the correct SSL_CTX is assigned
>  with SSL_use_SSL_CTX
>   the private key data is retrieved just once once, cached, and installed
> into the
> SSL_CTX;
>   this is done in the worker process
> - the PEM data has an arbitrary limit of 16384 bytes
> 

Hello Richard,

I'd rather not add another cache on top of the current cache system, it
will complexify the loading and we are trying to simplify it.

Did you identify why the fork was causing an issue? We should probably
try to understand this before, it could be something stupid in haproxy's
code or in the pkcs11 provider.

For 3.1 I plan to move the configuration loading in the worker so you
won't have this problem anymore. We still need to validate that
everything will be compatible but I have good hope that it's doable and
will cleanup a lot of startup code.

There is an experimental branch there
https://github.com/haproxy/haproxy/commits/20240131-mworkerv3-rewrite/
It does not do much for now but it could be enough to test the startup
with an HSM. I'll try to test this when I have some time.

Regards,

-- 
William Lallemand

Re: About the SPOE

[Christopher Faulet]

Thanks Lokesh, Abhijeet and Aleksandar for your feedback. This truly help us.
Thanks too to Pierre and Mattia for their feedback on the request mirroring.
Rest assured that we take this into account in our reflections.

After some internal discussions and also regarding to feedback we had
internally, we've decided to invest some time to rewrite the engine for the
3.1. A feature request was created to keep the conversation going
(https://github.com/haproxy/haproxy/issues/2502).

But to sum up, the idea is to rethink the engine to make it simpler. Of course,
the engine will be based on recent core features. But features will also be
reduced to remain maintainable. Among other things, the asynchronous mode will
be removed because it is far too complex and most probably unused. It is clearly
an over-designed feature. Then we will not invest time on the payload streaming
support. This feature is pretty complex to implement and this is mainly why we
never made the current SPOE evolved. Let's be reasonable this time. This will 
probably influence the design.


For people who have invested time in SPOAs development, the protocol will be
kept compatible. The purpose here is to rewrite the engine itself. So it is
above all internal to HAProxy. For the 3.0, the warning about the SPOE will be
changed to notify users some configuration changes should be expected in future
versions. Some other small changes should be expected for the 3.0. But the heavy
lifting will be performed on the 3.1.

Of course, it is still a subject under discussion. The above issue is here to
collect ideas for the next steps but also for more long term features. Feel free
to feed it.

Regards,
--
Christopher Faulet

Re: [PATCH] BUG/MINOR: server: fix persistence cookie for dynamic servers

[Amaury Denoyelle]

On Wed, Mar 27, 2024 at 02:34:25PM +, Damien Claisse wrote:
> When adding a server dynamically, we observe that when a backend has a
> dynamic persistence cookie, the new server has no cookie as we receive
> the following HTTP header:
> set-cookie: test-cookie=; Expires=Thu, 01-Jan-1970 00:00:01 GMT; path=/
> Whereas we were expecting to receive something like the following, which
> is what we receive for a server added in the config file:
> set-cookie: test-cookie=abcdef1234567890; path=/
> After investigating code path, srv_set_dyncookie() is never called when
> adding a server through CLI, it is only called when parsing config file
> or using "set server bkd1/srv1 addr".
> To fix this, call srv_set_dyncookie() inside cli_parse_add_server().
> This patch must be backported up to 2.4.
> ---
>  src/server.c | 5 +
>  1 file changed, 5 insertions(+)
> diff --git a/src/server.c b/src/server.c
> index 555cae82c..a93798f03 100644
> --- a/src/server.c
> +++ b/src/server.c
> @@ -5732,6 +5732,11 @@ static int cli_parse_add_server(char **args, char 
> *payload, struct appctx *appct
>*/
>   srv->rid = (srv_id_reuse_cnt) ? (srv_id_reuse_cnt / 2) : 0;
>  
> + /* generate new server's dynamic cookie if enabled on backend */
> + if (be->ck_opts & PR_CK_DYNAMIC) {
> + srv_set_dyncookie(srv);
> + }
> +
>   /* adding server cannot fail when we reach this:
>* publishing EVENT_HDL_SUB_SERVER_ADD
>*/
> -- 
> 2.34.1
> 

Thank you very much. This was merged in our development tree. In the
meantime, I also enabled "cookie" keyword for dynamic servers as nothing
prevented it.

-- 
Amaury Denoyelle

Re: Help tracking "connection refused" under pressure on v2.9

[Willy Tarreau]

On Wed, Mar 27, 2024 at 02:26:47PM -0300, Ricardo Nabinger Sanchez wrote:
> On Wed, 27 Mar 2024 11:06:39 -0300
> Felipe Wilhelms Damasio  wrote:
> 
> > kernel: traps: haproxy[2057993] trap invalid opcode ip:5b3e26
> > sp:7fd7c002f100 error:0 in haproxy[42c000+1f7000]
> 
> We managed to get a core file, and so created ticket #2508
> (https://github.com/haproxy/haproxy/issues/2508) with more details.

Thanks guys! So there seems to be an annoying bug. However I'm not sure
how this is related to your "connection refused", except if you try to
connect at the moment the process crashes and restarts, of course. I'm
seeing that the bug here is stktable_requeue_exp() calling task_queue()
with an invalid task expiration. I'm having a look now. I'll respond in
the issue with what I can find, thanks for your report.

Since you were speaking about FD count and maxconn at 900k, please let
me take this opportunity for a few extra sanity checks. By default we
assign up to about 50% of the FD to pipes (i.e. up to 25% pipes compared
to connections), so if maxconn is 900k you can reach 1800 + 900 = 2700k
FD. One thing to keep in mind is that /proc/sys/fs/nr_open sets a
per-process hard limit and usually is set to 1M, and that
/proc/sys/fs/file-max sets a system-wide limit and depends on the amount
of RAM, so both may interact with such a large setting. We could for
example imagine that at ~256k connections with as many pipes you're
reaching around 1M FDs and that the connection from socat to the CLI
socket cannot be accepted and is rejected. Since you recently updated
your kernel, it might be worth checking if the default values are still
in line with your usage.

Cheers,
Willy

Re: Help tracking "connection refused" under pressure on v2.9

[Ricardo Nabinger Sanchez]

On Wed, 27 Mar 2024 11:06:39 -0300
Felipe Wilhelms Damasio  wrote:

> kernel: traps: haproxy[2057993] trap invalid opcode ip:5b3e26
> sp:7fd7c002f100 error:0 in haproxy[42c000+1f7000]

We managed to get a core file, and so created ticket #2508
(https://github.com/haproxy/haproxy/issues/2508) with more details.

Cheers,

-- 
Ricardo Nabinger Sanchez https://www.taghos.com.br/

Re: Help tracking "connection refused" under pressure on v2.9

[Ricardo Nabinger Sanchez]

On Wed, 27 Mar 2024 11:06:39 -0300
Felipe Wilhelms Damasio  wrote:

> kernel: traps: haproxy[2057993] trap invalid opcode ip:5b3e26 sp:7fd7c002f100 
> error:0 in haproxy[42c000+1f7000]

In our build, this would be where instruction pointer was:

(gdb) list *0x5b10e6
0x5b10e6 is in __task_queue (src/task.c:285).
280(wq != _ctx->timers && wq != _ctx->timers));
281 #endif
282 /* if this happens the process is doomed anyway, so better 
catch it now
283  * so that we have the caller in the stack.
284  */
285 BUG_ON(task->expire == TICK_ETERNITY);
286
287 if (likely(task_in_wq(task)))
288 __task_unlink_wq(task);
289

However, we can't produce a stack trace from only the instruction
pointer; at least I don't know how (but would love to learn if it is
possible).

We are trying to get a core dump, too.

Cheers,

-- 
Ricardo Nabinger Sanchez https://www.taghos.com.br/

Re: Help tracking "connection refused" under pressure on v2.9

[Felipe Wilhelms Damasio]

Hi,

We've confirmed a few findings after we poured ~75-80Gbps of traffic
on purpose on a single machine:
- haproxy does indeed crashes;
- hence, we have no stats socket to collect a few things;

It seems that under pressure (not sure which conditions yet) the
kernel seems to be killing it. dmesg shows:

kernel: traps: haproxy[2057993] trap invalid opcode ip:5b3e26
sp:7fd7c002f100 error:0 in haproxy[42c000+1f7000]

This is a relatively new kernel:

Linux ndt-spo-12 6.1.60 #1 SMP PREEMPT_DYNAMIC Wed Oct 25 19:17:36 UTC
2023 x86_64 Intel(R) Xeon(R) Gold 6338N CPU @ 2.20GHz GenuineIntel
GNU/Linux

And it seems to happen on different kernels.

Does anyone have any tips on how to proceed to track this down?

Before the crash, "show info" showed only around 27,000 CurConn, so
not a great deal for maxconn 90.

Thanks!

On Tue, Mar 26, 2024 at 11:33 PM Felipe Wilhelms Damasio
 wrote:
>
> Hi,
>
> Since we don't really know how to track this one, we thought it might
> be better to reach out here to get feedback.
>
> We're using haproxy to deliver streaming files under pressure
> (80-90Gbps per machine). When using h1/http, splice-response is a
> great help to keep load under control. We use branch v2.9 at the
> moment.
>
> However, we've hit a bug with splice-response (Github issue created)
> and we had to use all day our haproxies without splicing.
>
> When we reach a certain load, a "connection refused" alarm starting
> buzzing like crazy (2-3 times every 30 minutes). This alarm is simply
> a connect to localhost with 500ms timeout:
>
> socat /dev/null  tcp4-connect:127.0.0.1:80,connect-timeout=0.5
>
> The log file indicates the port is virtually closed:
>
> 2024/03/27 01:06:04 socat[984480] E read(6, 0xe98000, 8192): Connection 
> refused
>
> The thing is haproxy process is very much alive...so we just restart
> it everytime this happens.
>
> What data do you suggest we collect to help track this down? Not sure
> if the stats socket is available, but we can definitely try and get
> some information.
>
> We're not running out of fds, or even connections with/without backlog
> (we have a global maxconn of 90 with ~30,000 streaming sessions
> active and we have tcp_max_syn_backlog set to 262144), we checked. But
> it seems to correlate with heavy traffic.
>
> Thanks!
>
> --
> Felipe Damasio



-- 
Felipe Damasio