Re: FCGI calls return 500 with "IH" Stream State

[Aleksandar Lazic]

Hi.

I have added fcgi trace

```
global
  log stdout format raw daemon debug

  pidfile /data/haproxy/run/haproxy.pid
  # maxconn  auto config from hap
  # nbthread auto config from hap

  master-worker

  #tune.comp.maxlevel 5

  expose-experimental-directives
  trace fcgi sink stdout
  trace fcgi verbosity advanced
  trace fcgi event any
  trace fcgi start now

    # turn on stats unix socket
  stats socket /data/haproxy/run/stats mode 660 level admin expose-fd listeners

```

and created with that output a issue.

https://github.com/haproxy/haproxy/issues/2568

Regards

Alex

On 2024-05-16 (Do.) 17:05, Aleksandar Lazic wrote:

Hi.

I have a strange behavior with HAProxy and FCGI PHP App.

When I call an admin URL returns HAProxy a 500, after a refresh of the same 
page returns the HAProxy 200.


```
10.128.2.35:39684 [16/May/2024:14:54:26.229] craft-cms fcgi-servers/craftcms1 
0/0/0/-1/1138 500 15416 - - IH-- 2/2/0/0/0 0/0 "GET /craftcms/admin/settings 
HTTP/1.1"


10.131.0.26:46546 [16/May/2024:14:56:01.870] craft-cms fcgi-servers/craftcms1 
0/0/0/1511/1514 200 113460 - -  2/2/0/0/0 0/0 "GET 
/craftcms/admin/settings HTTP/1.1"

```

How can I debug this 'I' flag which should never happen as the doc say.

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

```
    I : an internal error was identified by the proxy during a self-check.
    This should NEVER happen, and you are encouraged to report any log
    containing this, because this would almost certainly be a bug. It
    would be wise to preventively restart the process after such an
    event too, in case it would be caused by memory corruption.
```

I use the latest haproxy image haproxytech/haproxy-ubuntu:2.9 in OpenShift 
with that config.


```
global
    log stdout format raw daemon debug

    pidfile /data/haproxy/run/haproxy.pid
    # maxconn  auto config from hap
    # nbthread auto config from hap

    master-worker

    tune.comp.maxlevel 5

    # turn on stats unix socket
    stats socket /data/haproxy/run/stats mode 660 level admin expose-fd 
listeners

resolvers kube-dns
  nameserver dns1 dns-default.openshift-dns.svc.cluster.local:53
  accepted_payload_size 4096
  resolve_retries   3
  timeout resolve   1s
  timeout retry 1s
  hold other   30s
  hold refused 30s
  hold nx  30s
  hold timeout 30s
  hold valid   10s
  hold obsolete    30s

defaults
    mode    http
    balance leastconn
    log global
    option  httplog
    option  dontlognull
    option  log-health-checks
    option  forwardfor   except 10.196.106.108/32
    option  redispatch
    retries 3
    timeout http-request    10s
    timeout queue   30s
    timeout connect 10s
    timeout client  30s
    timeout server  30s
    timeout http-keep-alive 10s
    timeout check   10s
    #maxconn 3000

frontend craft-cms
  bind *:8080

  tcp-request inspect-delay 5s
  tcp-request content accept if HTTP

  # default check url from appgateway
  monitor-uri /health

  # https://www.haproxy.com/blog/load-balancing-php-fpm-with-haproxy-and-fastcgi
  # fix CVE-2019-11043
  http-request deny if { path_sub -i %0a %0d }

  # Mitigate CVE-2023-40225 (Proxy forwards malformed empty Content-Length 
headers)

  http-request deny if { hdr_len(content-length) 0 }

  # Strip off Proxy headers to prevent HTTpoxy (https://httpoxy.org/)
  http-request del-header Proxy

  # DNS labels are case insensitive (RFC 4343), we need to convert the 
hostname into lowercase
  # before matching, or any requests containing uppercase characters will 
never match.

  http-request set-header Host %[req.hdr(Host),lower]

  acl exist-php-ext path_sub -i .php
  acl fpm-status path /fpm-status

  http-request set-path /index.php%[path] if !exist-php-ext !fpm-status !{ 
path_end .php }


  # https://www.haproxy.com/blog/haproxy-and-http-strict-transport-security-hsts
  # max-age is mandatory
  # 1600 seconds is a bit more than 6 months
  http-response set-header Strict-Transport-Security "max-age=1600; 
includeSubDomains; preload;"


  default_backend fcgi-servers

listen stats
  bind *:1936

  # Health check monitoring uri.
  monitor-uri /healthz

  # provide prometheus endpoint
  http-request use-service prometheus-exporter if { path /metrics }

  # Add your custom health check monitoring failure condition here.
  # monitor fail if 
  stats enable
  stats uri /

backend fcgi-servers

  option httpchk
  http-check connect proto fcgi
  http-check send meth GET uri /fpm-ping

  use-fcgi-app php-fpm

  # https://www.haproxy.com/blog/circuit-breaking-haproxy
  server-template craftcms 10 
"${CRAFT_SERVICE}.${NAMESPACE}.svc.cluster.local":9000 proto fcgi check 
resolvers kube-dns init-addr none 

Re: [PATCH] DOC: Update UUID references to RFC 9562

[Willy Tarreau]

On Sun, May 12, 2024 at 05:08:34PM +0200, Tim Duesterhus wrote:
> When support for UUIDv7 was added in commit
> aab6477b67415c4cc260bba5df359fa2e6f49733
> the specification still was a draft.
> 
> It has since been published as RFC 9562.

Excellent timing ;-)

Now merged, thank you Tim!
Willy

Re: Meeting with LML Studios

[M Sami Kerrouche]

Hi Guys,

Was this something you needed?

I'd appreciate an answer. Happy to help.

Best wishes,

Sami

On Mon, May 6, 2024 at 1:10 PM M Sami Kerrouche <
s...@londonmedialounge.co.uk> wrote:

> Hi,
>
> I am waiting for you on our call that you booked.
>
> Let me know if you'd like to reschedule.
>
> Here's my phone number, call me for a quicker response.
>
> +44 7577 295184
>
> Best wishes,
>
> *Sami Kerrouche*
> Managing Director
>
> London Media Lounge Ltd
> 16a/17a Windsor St
> Uxbridge
> Middx
> UB8 1AB
> www.LondonMediaLounge.co.uk 
> https://www.linkedin.com/in/msamikerrouche/
> 
>

Re: [PATCH v2] MINOR: config: rhttp: Don't require SSL when attach-srv name parsing

[Amaury Denoyelle]

On Wed, May 08, 2024 at 11:43:11AM +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)
> while a rhttp server line usually looks like this:
> server srv rhttp@ sni req.hdr(host)
> The server sni argument is used as a key for looking up connection in the
> connection pool.  The attach-srv name argument is used as a key for
> inserting connections into the pool.  For it to work correctly they must
> match.  There was a check that either both the attach-srv and server
> provide that key or neither does.
> It also checked that SSL and SNI was activated on the server.  This is too
> strict.  This patch removes that requirement.  Now you can pass arbitrary
> expressions as the name expression.
> With this patch we also produce a more helpful and specific error message.
> I'm doing this as I want to use `fc_pp_unique_id` as the name.
> Arguably it would be easier to understand if instead of using `name` and
> `sni` for `attach-srv` and `server` rules it used the same term in both
> places - like "conn-pool-key" or something.  That would make it clear that
> the two must match.  But it's too late to change that now.
> [...]

Many thanks for your new patch. I have just merged it with only small
adaptation in the commit message. Note that however I tried to use PROXY
protocol with reverse HTTP to test it, but it appears that for now it is
impossible due to several haproxy internal limitations.

The main issue is that send-proxy and other related options on a server
line used for reverse HTTP are simply ignored, thus you cannot connect
to a bind with accept-proxy. I managed to fix this, but this is not
enough as PROXY protocol in LOCAL mode seems to not work completely as
expected either...

-- 
Amaury Denoyelle

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

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

пн, 13 мая 2024 г. в 11:29, William Lallemand :

> On Thu, May 09, 2024 at 10:24:55PM +0200, Илья Шипицин wrote:
> > sorry for th delay.
> >
> > indeed, it's better to drop asan redirection. I sent a patch to the list.
> >
> > for my defence I can say that in my experiments asan.log worked as
> expected
> > :)
> >
>
> No worries, we had a change of distribution version since then, maybe
> the variable need more parameters to achieve it with this version, no
> idea. But honestly I find it simpler to have the ASAN errors in the
> reg-test directly.
>

initially I thought of using "asan_options=halt_on_error=0"
actually, I believed that it is default behaviour (it is not), even more,
switching to halt_on_error=0 required asan lib rebuild.


>
> > ignore show version error · chipitsine/haproxy@2900d66 (github.com)
> > <
> https://github.com/chipitsine/haproxy/actions/runs/9022839976/job/24793325629
> >
> >
> > [image: image.png]
>
> I never saw this at all, I doubt it worked in master for a long time :-)
>
> https://github.com/haproxy/haproxy/actions/runs/9060411631/job/24890056499
>
> That's better indeed, I'll merge the patch. Thanks!
>
>
> --
> William Lallemand
>

Re: [PATCH 0/1] CI: drop asan.log umbrella for good

[William Lallemand]

On Thu, May 09, 2024 at 10:19:17PM +0200, Ilia Shipitsin wrote:
> for some reasons it appeared to be a good idea
> to collect ASAN log separately from VTest error logs,
> but also it appeared to work poorly in real life (compared to
> specially prepared synthetic environments).
> 
> let drop asan.log redirection
> 
> Ilia Shipitsin (1):
>   CI: drop asan.log umbrella completely
> 
>  .github/workflows/vtest.yml | 9 -
>  1 file changed, 9 deletions(-)
> 

Merged, thanks!

-- 
William Lallemand

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

[William Lallemand]

On Thu, May 09, 2024 at 10:24:55PM +0200, Илья Шипицин wrote:
> sorry for th delay.
> 
> indeed, it's better to drop asan redirection. I sent a patch to the list.
> 
> for my defence I can say that in my experiments asan.log worked as expected
> :)
> 

No worries, we had a change of distribution version since then, maybe
the variable need more parameters to achieve it with this version, no
idea. But honestly I find it simpler to have the ASAN errors in the
reg-test directly.

> ignore show version error · chipitsine/haproxy@2900d66 (github.com)
> 
> 
> [image: image.png]

I never saw this at all, I doubt it worked in master for a long time :-)

https://github.com/haproxy/haproxy/actions/runs/9060411631/job/24890056499

That's better indeed, I'll merge the patch. Thanks!


-- 
William Lallemand

AW: [EXT] Re: error HAproxy with Galera Cluster v4

[Marno Krahmer]

Hey,

There actually is some stuff in the haproxy documentation about this:
https://docs.haproxy.org/2.9/configuration.html#4-option%20mysql-check

MySQL will block a client host when it does more unsuccessful authentication 
requests than configured in the global variable “max_connect_errors”.

This can happen when you do health check more frequently than “real” MySQL 
connection come it.

You can change the value of max_connect_errors according to the documentation: 
https://dev.mysql.com/doc/refman/8.0/en/server-system-variables.html#sysvar_max_connect_errors

Running a “FLUSH HOSTS;” on the affected MySQL node will (temporarily) solve 
that problem too.

If you don’t want to change that variable, you can either decrease the healh 
check interval, or could use a different health check mechanism.

In our company, we use a small script running on every MySQL-Node, that exposes 
an HTTP-Enpoint, reporting the MySQL-state.
Then haproxy is making a HTTP-Request for monitoring and allows us to configure 
expected response code & content.

Cheers
Marno


Von: Willy Tarreau 
Datum: Freitag, 10. Mai 2024 um 14:28
An: Iglesias Paz, Jaime 
Cc: haproxy@formilux.org 
Betreff: [EXT] Re: error HAproxy with Galera Cluster v4
Hello,

On Fri, May 10, 2024 at 12:00:17PM +, Iglesias Paz, Jaime wrote:
> Hey guys, I have a problem with HAProxy and Galera Cluster v4 MySQL (3 
> nodes). I boot the HAProxy server and it returns the following error:
>
> may 10 13:48:20 phaproxysql1 haproxy[661]: Proxy stats started.
> may 10 13:48:20 phaproxysql1 haproxy[661]: Proxy stats started.
> may 10 13:48:20 phaproxysql1 haproxy[661]: [NOTICE] 130/134820 (661) : New 
> worker #1 (663) forked
> may 10 13:48:20 phaproxysql1 systemd[1]: Started HAProxy Load Balancer.
> may 10 13:48:20 phaproxysql1 haproxy[663]: [WARNING] 130/134820 (663) : 
> Server galeramanagerprd/nodo1prd is DOWN, reason: Layer7 wrong status, code: 
> 1129, info: "Host 'X' is blocked because of many connection errors; 
> unblock>
> may 10 13:48:21 phaproxysql1 haproxy[663]: [WARNING] 130/134821 (663) : 
> Server galeramanagerprd/nodo2prd is DOWN, reason: Layer7 wrong status, code: 
> 1129, info: "Host 'X' is blocked because of many connection errors; 
> unblock>
> may 10 13:48:21 phaproxysql1 haproxy[663]: [WARNING] 130/134821 (663) : 
> Server galeramanagerprd/nodo3prd is DOWN, reason: Layer7 wrong status, code: 
> 1129, info: "Host '' is blocked because of many connection errors; 
> unblock>
> may 10 13:48:21 phaproxysql1 haproxy[663]: [NOTICE] 130/134821 (663) : 
> haproxy version is 2.2.9-2+deb11u6
> may 10 13:48:21 phaproxysql1 haproxy[663]: [NOTICE] 130/134821 (663) : path 
> to executable is /usr/sbin/haproxy
> may 10 13:48:21 phaproxysql1 haproxy[663]: [ALERT] 130/134821 (663) : proxy 
> 'galeramanagerprd' has no server available!
>
> The haproxy.cfg configuration file:
> 
> defaults
> log global
> modehttp
> option  httplog
> option  dontlognull
> timeout connect 5000
> timeout client  5
> timeout server  5
> errorfile 400 /etc/haproxy/errors/400.http
> errorfile 403 /etc/haproxy/errors/403.http
> errorfile 408 /etc/haproxy/errors/408.http
> errorfile 500 /etc/haproxy/errors/500.http
> errorfile 502 /etc/haproxy/errors/502.http
> errorfile 503 /etc/haproxy/errors/503.http
> errorfile 504 /etc/haproxy/errors/504.http
>
> listen galeramanagerprd
> bind *:3306
> balance source
> mode tcp
> #option tcplog
> option tcpka
> option mysql-check user haproxy
> server nodo1prd X:3306 check weight 1
> server nodo2prd X:3306 check weight 1
> server nodo3prd X:3306 check weight 1
> 
>
> (*) for security I change the IPs to X
>
> Reviewing the documentation I can't find where the problem may be.

That reminds me of something a long time ago, where there was a limit on
the number of check a mysql server would accept from a same IP address,
and it was necessary to change the setting to unlimited. I don't remember
the details but there was something to do using some insert commands. I
don't know if this is still needed after all these years, but the error
message strongly suggests something like this.

Willy

Re: error HAproxy with Galera Cluster v4

[Willy Tarreau]

Hello,

On Fri, May 10, 2024 at 12:00:17PM +, Iglesias Paz, Jaime wrote:
> Hey guys, I have a problem with HAProxy and Galera Cluster v4 MySQL (3 
> nodes). I boot the HAProxy server and it returns the following error:
> 
> may 10 13:48:20 phaproxysql1 haproxy[661]: Proxy stats started.
> may 10 13:48:20 phaproxysql1 haproxy[661]: Proxy stats started.
> may 10 13:48:20 phaproxysql1 haproxy[661]: [NOTICE] 130/134820 (661) : New 
> worker #1 (663) forked
> may 10 13:48:20 phaproxysql1 systemd[1]: Started HAProxy Load Balancer.
> may 10 13:48:20 phaproxysql1 haproxy[663]: [WARNING] 130/134820 (663) : 
> Server galeramanagerprd/nodo1prd is DOWN, reason: Layer7 wrong status, code: 
> 1129, info: "Host 'X' is blocked because of many connection errors; 
> unblock>
> may 10 13:48:21 phaproxysql1 haproxy[663]: [WARNING] 130/134821 (663) : 
> Server galeramanagerprd/nodo2prd is DOWN, reason: Layer7 wrong status, code: 
> 1129, info: "Host 'X' is blocked because of many connection errors; 
> unblock>
> may 10 13:48:21 phaproxysql1 haproxy[663]: [WARNING] 130/134821 (663) : 
> Server galeramanagerprd/nodo3prd is DOWN, reason: Layer7 wrong status, code: 
> 1129, info: "Host '' is blocked because of many connection errors; 
> unblock>
> may 10 13:48:21 phaproxysql1 haproxy[663]: [NOTICE] 130/134821 (663) : 
> haproxy version is 2.2.9-2+deb11u6
> may 10 13:48:21 phaproxysql1 haproxy[663]: [NOTICE] 130/134821 (663) : path 
> to executable is /usr/sbin/haproxy
> may 10 13:48:21 phaproxysql1 haproxy[663]: [ALERT] 130/134821 (663) : proxy 
> 'galeramanagerprd' has no server available!
> 
> The haproxy.cfg configuration file:
> 
> defaults
> log global
> modehttp
> option  httplog
> option  dontlognull
> timeout connect 5000
> timeout client  5
> timeout server  5
> errorfile 400 /etc/haproxy/errors/400.http
> errorfile 403 /etc/haproxy/errors/403.http
> errorfile 408 /etc/haproxy/errors/408.http
> errorfile 500 /etc/haproxy/errors/500.http
> errorfile 502 /etc/haproxy/errors/502.http
> errorfile 503 /etc/haproxy/errors/503.http
> errorfile 504 /etc/haproxy/errors/504.http
> 
> listen galeramanagerprd
> bind *:3306
> balance source
> mode tcp
> #option tcplog
> option tcpka
> option mysql-check user haproxy
> server nodo1prd X:3306 check weight 1
> server nodo2prd X:3306 check weight 1
> server nodo3prd X:3306 check weight 1
> 
> 
> (*) for security I change the IPs to X
> 
> Reviewing the documentation I can't find where the problem may be.

That reminds me of something a long time ago, where there was a limit on
the number of check a mysql server would accept from a same IP address,
and it was necessary to change the setting to unlimited. I don't remember
the details but there was something to do using some insert commands. I
don't know if this is still needed after all these years, but the error
message strongly suggests something like this.

Willy

Re: some QUIC questions

[Amaury Denoyelle]

On Mon, May 06, 2024 at 08:16:34PM +0200, Björn Jacke wrote:
> On 06.05.24 15:34, Shawn Heisey wrote:
> > On 5/6/24 06:02, Björn Jacke wrote:
> > > frontend ft_443
> > >    bind :::443 ssl crt /ssl/combined.pem
> > >    bind quic6@:443 ssl crt /ssl/combined.pem alpn h3
> > >    option tcp-smart-accept
> > >    http-after-response add-header alt-svc 'h3=":443"; ma=600;
> > > persistent=1'
> > > 
> > > > frontend ft_quic_test
> > >  mode tcp
> > >  bind quic6@:443 ssl crt /ssl/combined.pem
> > >  use_backend local_smb
> > > > > this results in this config check error thoug:
> > > > > [ALERT]    (3611777) : config : frontend 'ft_quic_test' : MUX
> > > protocol 'quic' is not usable for 'bind quic6@:443' at
> > > [/etc/haproxy/ haproxy.cfg:73].
> > > > > So a setup like this is not supported by HAProxy's QUIC
> > > implementation currently, right? Is QUIC in HAProxy HTTP3 only for
> > > now?\
> > > The alpn on the first config snippet only includes h3, not quic.  Here
> > are alpn and npn settings that allow some of the quic protocol
> > variations as well as h3 itself:
> for the http frontend sniplet h3 as only alpn protocol was intended. It
> turned out to be a firewall causing making haproxy "ignore" the incoming
> quic traffic, sorry for not finding that earlier.
> Continuing the test with connection migration on network changes showed that
> connection migration is not working. I'm not sure though if none of the
> browsers do really support this or if this not being supported on the
> haproxy server side. Does any of the QUIC experts here have some insights on
> that?

Indeed haproxy currently does not implement connection migration.  For
now, haproxy will silently ignored datagrams received for a known
connection on another network endpoint.  We have started to talk about
it so it's definitely on our roadmap. But first we have to update our
documentation to reflect the current state.

> > The second one is a tcp frontend ... I feel pretty sure that h3/quic
> > requires http in the frontend, not tcp.
> but for any non-http protocol using QUIC as transport layer a http type
> frontend is obviously not the right choice.
> So let me ask the question differently: is QUIC support in haproxy currently
> only targeting at http3 support or is generic QUIC transport also on the
> agenda? From the docs I can't find much about non-http related QUIC support.

For the moment, QUIC in haproxy is only usable in HTTP mode with either
HTTP/3 or HTTP/0.9. The last one is reserved for testing for interop
testing and ensure we can use multiple application protocols. As such,
it could be possible to support non-http protocols without too much
work. The biggest constraint for now is the lack of testing client. But
if have a working setup for this, do not hesitate to share it with us it
would be extremely useful.

-- 
Amaury Denoyelle

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

[Willy Tarreau]

On Wed, May 08, 2024 at 01:19:22PM +, Dorian Craps wrote:
> first of all, thank you for your interest.
> 
> I already made a version with an option to enable MPTCP
> -https://github.com/CrapsDorian/haproxy/pull/1
> 
> I'm working on a new version with "mptcp@address" as Willy requested.

OK, thank you Dorian. I'm sorry not to have more time to assign to
review your work at the moment, it's really a matter of bad timing :-/

Willy

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

[Dorian Craps]

first of all, thank you for your interest.

I already made a version with an option to enable MPTCP
-https://github.com/CrapsDorian/haproxy/pull/1

I'm working on a new version with "mptcp@address" as Willy requested.

Dorian

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

[William Manley]

On Thu, Apr 25, 2024, at 2:07 PM, Amaury Denoyelle wrote:
> Sorry for the delay. We have rediscussed this issue this morning and
> here is my answer on your patch.

Sorry for the even larger delay in responding :).  Thanks for looking at this.

> 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.

I'll fix this.

> > ---
> >  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.

I agree.  Did you see my replacement patch "MINOR: config: rhttp: Don't require 
SSL when attach-srv name parsing" 
https://www.mail-archive.com/haproxy@formilux.org/msg44826.html .  It 
implements this general idea, but with a nice specific error message depending 
on whether sni or name is missing.

That replacement patch still needs the commit message corrected as you remarked 
above, so I'll send a v2 of that patch.

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: How to configure DH groups for TLS 1.3

[Tristan]

Hi Dominik,


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”.

[snip]
While Lukas answered the specific question better than I could, does the 
hardening guide you're following happen to be a public resource in general?


Good public guidelines on the topic is very sparse [1], and I'd be 
interested in these if they exist somewhere, if only out of curiosity.


Regards,
Tristan

[1]: Or often essentially nonexistent, short of reading dozens of papers 
off arxiv, of which the majority seem to focus on PoCs rather than 
practical advice

Re: [PR] fix show-sess-to-flags.sh cob fd state

[Willy Tarreau]

Hi!

On Tue, May 07, 2024 at 02:23:02AM +, PR Bot wrote:
> Author: zhibin.zhu 
> Number of patches: 1
> 
> This is an automated relay of the Github pull request:
>fix show-sess-to-flags.sh cob fd state
(...)

> From 95be08c6f4f382ec1b0e34765d4c1f09ddcdebb6 Mon Sep 17 00:00:00 2001
> From: "zhibin.zhu" 
> Date: Wed, 1 May 2024 18:51:26 +0800
> Subject: [PATCH] fix show-sess-to-flags.sh cob fd state
> 
> ---
>  dev/flags/show-sess-to-flags.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/dev/flags/show-sess-to-flags.sh b/dev/flags/show-sess-to-flags.sh
> index 79003a4f25d8..b352709f3a39 100755
> --- a/dev/flags/show-sess-to-flags.sh
> +++ b/dev/flags/show-sess-to-flags.sh
> @@ -195,7 +195,7 @@ while read -r; do
>  ! [[ "$REPLY" =~ [[:blank:]]h2c.*\.flg=([0-9a-fx]*) ]] || 
> append_flag b.h2c.flg   h2c  "${BASH_REMATCH[1]}"
>  elif [ $ctx = cob ]; then
>  ! [[ "$REPLY" =~ [[:blank:]]flags=([0-9a-fx]*) ]]  || 
> append_flag b.co.flgconn "${BASH_REMATCH[1]}"
> -! [[ "$REPLY" =~ [[:blank:]]fd.state=([0-9a-fx]*) ]]   || 
> append_flag b.co.fd.st  fd   "${BASH_REMATCH[1]}"
> +! [[ "$REPLY" =~ [[:blank:]]fd.state=([0-9a-fx]*) ]]   || 
> append_flag b.co.fd.st  fd   0x"${BASH_REMATCH[1]#0x}"
>  elif [ $ctx = res ]; then
>  ! [[ "$REPLY" =~ [[:blank:]]\(f=([0-9a-fx]*) ]]|| 
> append_flag res.flg chn  "${BASH_REMATCH[1]}"
>  ! [[ "$REPLY" =~ [[:blank:]]an=([0-9a-fx]*) ]] || 
> append_flag res.ana ana  "${BASH_REMATCH[1]}"

Hmm why, what is the problem it tries to solve ? Maybe we don't always
emit the 0x on the output ? Please try to provide a bit of info that
can be added to the commit message.

Thanks!
Willy

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

[David CARLIER]

hi and sorry for the long reply.

I will let you know once it is officially release, it needs to pass our QA
test still.

Kind regards.

On Mon, 6 May 2024 at 22:52, Mahendra Patil 
wrote:

> any update when we can get  3.2.3 release
>
> On Wed, Apr 3, 2024 at 10:51 AM David CARLIER  wrote:
>
>> 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

[Mahendra Patil]

any update when we can get  3.2.3 release

On Wed, Apr 3, 2024 at 10:51 AM David CARLIER  wrote:

> 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: some QUIC questions

[Björn Jacke]

On 06.05.24 15:34, Shawn Heisey wrote:

On 5/6/24 06:02, Björn Jacke wrote:

frontend ft_443
   bind :::443 ssl crt /ssl/combined.pem
   bind quic6@:443 ssl crt /ssl/combined.pem alpn h3
   option tcp-smart-accept
   http-after-response add-header alt-svc 'h3=":443"; ma=600; 
persistent=1'





frontend ft_quic_test
 mode tcp
 bind quic6@:443 ssl crt /ssl/combined.pem
 use_backend local_smb

this results in this config check error thoug:

[ALERT]    (3611777) : config : frontend 'ft_quic_test' : MUX protocol 
'quic' is not usable for 'bind quic6@:443' at [/etc/haproxy/ 
haproxy.cfg:73].


So a setup like this is not supported by HAProxy's QUIC implementation 
currently, right? Is QUIC in HAProxy HTTP3 only for now?\


The alpn on the first config snippet only includes h3, not quic.  Here 
are alpn and npn settings that allow some of the quic protocol 
variations as well as h3 itself:


for the http frontend sniplet h3 as only alpn protocol was intended. It 
turned out to be a firewall causing making haproxy "ignore" the incoming 
quic traffic, sorry for not finding that earlier.


Continuing the test with connection migration on network changes showed 
that connection migration is not working. I'm not sure though if none of 
the browsers do really support this or if this not being supported on 
the haproxy server side. Does any of the QUIC experts here have some 
insights on that?



The second one is a tcp frontend ... I feel pretty sure that h3/quic 
requires http in the frontend, not tcp.


but for any non-http protocol using QUIC as transport layer a http type 
frontend is obviously not the right choice.


So let me ask the question differently: is QUIC support in haproxy 
currently only targeting at http3 support or is generic QUIC transport 
also on the agenda? From the docs I can't find much about non-http 
related QUIC support.


Thank you
Björn

Re: some QUIC questions

[Shawn Heisey]

On 5/6/24 06:02, Björn Jacke wrote:

frontend ft_443
   bind :::443 ssl crt /ssl/combined.pem
   bind quic6@:443 ssl crt /ssl/combined.pem alpn h3
   option tcp-smart-accept
   http-after-response add-header alt-svc 'h3=":443"; ma=600; persistent=1'





frontend ft_quic_test
     mode tcp
     bind quic6@:443 ssl crt /ssl/combined.pem
     use_backend local_smb

this results in this config check error thoug:

[ALERT]    (3611777) : config : frontend 'ft_quic_test' : MUX protocol 
'quic' is not usable for 'bind quic6@:443' at [/etc/haproxy/ 
haproxy.cfg:73].


So a setup like this is not supported by HAProxy's QUIC implementation 
currently, right? Is QUIC in HAProxy HTTP3 only for now?\


The alpn on the first config snippet only includes h3, not quic.  Here 
are alpn and npn settings that allow some of the quic protocol 
variations as well as h3 itself:


alpn h3,h3-29,h3-28,h3-27 npn h3,h3-29,h3-28,h3-27

The second one is a tcp frontend ... I feel pretty sure that h3/quic 
requires http in the frontend, not tcp.


Thanks,
Shawn

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

[Björn Jacke]

Hi,

I came up a while ago with a patchset for MPTCP support for HAProxy 
also, see https://github.com/haproxy/haproxy/issues/1028


Back then I also discussed some ideas with Willy how to implement it 
more elegantly in HAProxy. It's still somewhere on my todo list but 
unfortunately I didn't catch that up since then. As I had a good 
real-world usecase recently for MPTCP I though of looking into this 
again also.


Björn

On 25.04.24 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. 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 1/3] BUILD: illumos: pthread_getcpuclockid is not available

[Willy Tarreau]

On Sun, May 05, 2024 at 01:43:33PM +0200,  ??? wrote:
> updated patches.

Cool, thanks, now applied.

> I'll address reorg to "compat.h" a bit later, once it is settled in my head

No worries, I've seen your other comment about the need to include
pthread.h, and this alone would be a good reason for leaving the test
where it is for now.

Willy

Re: [PATCH 1/3] BUILD: illumos: pthread_getcpuclockid is not available

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

updated patches.

I'll address reorg to "compat.h" a bit later, once it is settled in my head

вс, 5 мая 2024 г. в 12:48, Илья Шипицин :

> I will test and send simplified patch, i.e. I'll patch directly clock.c
>
> if we want to move that macro to compat.h, I'd postpone that for some
> investigation
>
> 1) we will need to include "pthread.h" from compat.h (currently it's not
> true)
> 2) we will need to make sure compat.h is included everywhere (I do not see
> that include in clock.c)
>
> вс, 5 мая 2024 г. в 12:24, Willy Tarreau :
>
>> On Sun, May 05, 2024 at 11:15:24AM +0200,  ??? wrote:
>> > ??, 5 ??? 2024 ?. ? 10:42, Willy Tarreau :
>> >
>> > > On Sun, May 05, 2024 at 09:12:41AM +0200, Miroslav Zagorac wrote:
>> > > > On 05. 05. 2024. 08:32, Willy Tarreau wrote:
>> > > > > On Sun, May 05, 2024 at 07:49:55AM +0200,  ??? wrote:
>> > > > >> ??, 5 ??? 2024 ?. ? 02:05, Miroslav Zagorac :
>> > > > >>> I think that this patch is not satisfactory because, for
>> example,
>> > > Solaris
>> > > > >>> 11.4.0.0.1.15.0 (from 2018) has _POSIX_TIMERS and
>> > > _POSIX_THREAD_CPUTIME
>> > > > >>> defined, but does not have the pthread_getcpuclockid() function;
>> > > while
>> > > > >>> solaris
>> > > > >>> 11.4.42.0.0.111.0 (from 2022) has that function.
>> > > > >>>
>> > > > >>
>> > > > >> I'm trying to build on this vmactions/solaris-vm: Use Solaris in
>> > > github
>> > > > >> actions 
>> > > > >> it does not have pthread_getcpuclockid()
>> > > > >
>> > > > > I'm wondering what the point of defining _POSIX_THREAD_CPUTIME
>> can be
>> > > > > then :-/
>> > > > >
>> > > >
>> > > > The pthread_getcpuclockid() function is declared in the include file
>> > > > /usr/include/pthread.h.  The only difference between the two
>> "versions"
>> > > of
>> > > > Solaris 11.4 is that the newer version has a declaration and the
>> older
>> > > one
>> > > > does not.
>> > > >
>> > > > However, _POSIX_THREAD_CPUTIME is defined in the
>> /usr/include/unistd.h
>> > > file as
>> > > > -1 in the UNIX 03 block of options that are not supported in Solaris
>> > > 11.4.
>> > > >
>> > > > /* Unsupported UNIX 03 options */
>> > > > #if defined(_XPG6)
>> > > > ..
>> > > > #define _POSIX_THREAD_CPUTIME (-1)
>> > > > ..
>> > > > #endif
>> > > >
>> > > >
>> > > > An explanation of that definition can be found here:
>> > > >
>> > > > https://docs.oracle.com/cd/E88353_01/html/E37842/unistd-3head.html
>> > > >
>> > > > "If a symbolic constant is defined with the value -1, the option is
>> not
>> > > > supported. Headers, data types, and function interfaces required
>> only
>> > > for the
>> > > > option need not be supplied. An application that attempts to use
>> anything
>> > > > associated only with the option is considered to be requiring an
>> > > extension.
>> > > (...)
>> > >
>> > > Ah excellent, that's quite useful! We're already doing that with
>> > > _POSIX_TIMERS. So I guess one just needs to try this instead:
>> > >
>> > > -#if defined(_POSIX_TIMERS) && (_POSIX_TIMERS > 0) &&
>> > > defined(_POSIX_THREAD_CPUTIME
>> > > +#if defined(_POSIX_TIMERS) && (_POSIX_TIMERS > 0) &&
>> > > defined(_POSIX_THREAD_CPUTIME && (_POSIX_THREAD_CPUTIME >= 0)
>> > >
>> >
>> > that worked (I added closing bracket after second "defined")
>>
>> Ah yes indeed. Thanks for the test. Do you want to update you patch maybe,
>> since you can test it ?
>>
>> Thanks,
>> Willy
>>
>
From 3cc9c51ae6b22beeaaee89d1e5f69b99c6874f39 Mon Sep 17 00:00:00 2001
From: Ilia Shipitsin 
Date: Sun, 5 May 2024 13:41:32 +0200
Subject: [PATCH 3/3] CI: netbsd: limit scheduled workflow to parent repo only

it is not very useful for most of forks.
---
 .github/workflows/netbsd.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.github/workflows/netbsd.yml b/.github/workflows/netbsd.yml
index ea0345954..6514725e1 100644
--- a/.github/workflows/netbsd.yml
+++ b/.github/workflows/netbsd.yml
@@ -7,6 +7,7 @@ on:
 jobs:
   gcc:
 runs-on: ubuntu-latest
+if: ${{ github.repository_owner == 'haproxy' }}
 permissions:
   contents: read
 steps:
-- 
2.43.0.windows.1

From 3c6b1a9dadfcefad4d94a1ee3a288a2e36865a52 Mon Sep 17 00:00:00 2001
From: Ilia Shipitsin 
Date: Sun, 5 May 2024 13:39:33 +0200
Subject: [PATCH 2/3] CI: add Illumos scheduled workflow

this is very initial build only implementation.
---
 .github/workflows/illumos.yml | 24 
 1 file changed, 24 insertions(+)
 create mode 100644 .github/workflows/illumos.yml

diff --git a/.github/workflows/illumos.yml b/.github/workflows/illumos.yml
new file mode 100644
index 0..b0b5e4852
--- /dev/null
+++ b/.github/workflows/illumos.yml
@@ -0,0 +1,24 @@
+name: Illumos
+
+on:
+  schedule:
+- cron: "0 0 25 * *"
+
+jobs:
+  gcc:
+runs-on: ubuntu-latest
+if: ${{ github.repository_owner == 'haproxy' }}
+permissions:
+  contents: read
+steps:
+  - name: "Checkout repository"
+uses: actions/checkout@v4
+
+  - 

Re: [PATCH 1/3] BUILD: illumos: pthread_getcpuclockid is not available

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

I will test and send simplified patch, i.e. I'll patch directly clock.c

if we want to move that macro to compat.h, I'd postpone that for some
investigation

1) we will need to include "pthread.h" from compat.h (currently it's not
true)
2) we will need to make sure compat.h is included everywhere (I do not see
that include in clock.c)

вс, 5 мая 2024 г. в 12:24, Willy Tarreau :

> On Sun, May 05, 2024 at 11:15:24AM +0200,  ??? wrote:
> > ??, 5 ??? 2024 ?. ? 10:42, Willy Tarreau :
> >
> > > On Sun, May 05, 2024 at 09:12:41AM +0200, Miroslav Zagorac wrote:
> > > > On 05. 05. 2024. 08:32, Willy Tarreau wrote:
> > > > > On Sun, May 05, 2024 at 07:49:55AM +0200,  ??? wrote:
> > > > >> ??, 5 ??? 2024 ?. ? 02:05, Miroslav Zagorac :
> > > > >>> I think that this patch is not satisfactory because, for example,
> > > Solaris
> > > > >>> 11.4.0.0.1.15.0 (from 2018) has _POSIX_TIMERS and
> > > _POSIX_THREAD_CPUTIME
> > > > >>> defined, but does not have the pthread_getcpuclockid() function;
> > > while
> > > > >>> solaris
> > > > >>> 11.4.42.0.0.111.0 (from 2022) has that function.
> > > > >>>
> > > > >>
> > > > >> I'm trying to build on this vmactions/solaris-vm: Use Solaris in
> > > github
> > > > >> actions 
> > > > >> it does not have pthread_getcpuclockid()
> > > > >
> > > > > I'm wondering what the point of defining _POSIX_THREAD_CPUTIME can
> be
> > > > > then :-/
> > > > >
> > > >
> > > > The pthread_getcpuclockid() function is declared in the include file
> > > > /usr/include/pthread.h.  The only difference between the two
> "versions"
> > > of
> > > > Solaris 11.4 is that the newer version has a declaration and the
> older
> > > one
> > > > does not.
> > > >
> > > > However, _POSIX_THREAD_CPUTIME is defined in the
> /usr/include/unistd.h
> > > file as
> > > > -1 in the UNIX 03 block of options that are not supported in Solaris
> > > 11.4.
> > > >
> > > > /* Unsupported UNIX 03 options */
> > > > #if defined(_XPG6)
> > > > ..
> > > > #define _POSIX_THREAD_CPUTIME (-1)
> > > > ..
> > > > #endif
> > > >
> > > >
> > > > An explanation of that definition can be found here:
> > > >
> > > > https://docs.oracle.com/cd/E88353_01/html/E37842/unistd-3head.html
> > > >
> > > > "If a symbolic constant is defined with the value -1, the option is
> not
> > > > supported. Headers, data types, and function interfaces required only
> > > for the
> > > > option need not be supplied. An application that attempts to use
> anything
> > > > associated only with the option is considered to be requiring an
> > > extension.
> > > (...)
> > >
> > > Ah excellent, that's quite useful! We're already doing that with
> > > _POSIX_TIMERS. So I guess one just needs to try this instead:
> > >
> > > -#if defined(_POSIX_TIMERS) && (_POSIX_TIMERS > 0) &&
> > > defined(_POSIX_THREAD_CPUTIME
> > > +#if defined(_POSIX_TIMERS) && (_POSIX_TIMERS > 0) &&
> > > defined(_POSIX_THREAD_CPUTIME && (_POSIX_THREAD_CPUTIME >= 0)
> > >
> >
> > that worked (I added closing bracket after second "defined")
>
> Ah yes indeed. Thanks for the test. Do you want to update you patch maybe,
> since you can test it ?
>
> Thanks,
> Willy
>

Re: [PATCH 1/3] BUILD: illumos: pthread_getcpuclockid is not available

[Willy Tarreau]

On Sun, May 05, 2024 at 11:15:24AM +0200,  ??? wrote:
> ??, 5 ??? 2024 ?. ? 10:42, Willy Tarreau :
> 
> > On Sun, May 05, 2024 at 09:12:41AM +0200, Miroslav Zagorac wrote:
> > > On 05. 05. 2024. 08:32, Willy Tarreau wrote:
> > > > On Sun, May 05, 2024 at 07:49:55AM +0200,  ??? wrote:
> > > >> ??, 5 ??? 2024 ?. ? 02:05, Miroslav Zagorac :
> > > >>> I think that this patch is not satisfactory because, for example,
> > Solaris
> > > >>> 11.4.0.0.1.15.0 (from 2018) has _POSIX_TIMERS and
> > _POSIX_THREAD_CPUTIME
> > > >>> defined, but does not have the pthread_getcpuclockid() function;
> > while
> > > >>> solaris
> > > >>> 11.4.42.0.0.111.0 (from 2022) has that function.
> > > >>>
> > > >>
> > > >> I'm trying to build on this vmactions/solaris-vm: Use Solaris in
> > github
> > > >> actions 
> > > >> it does not have pthread_getcpuclockid()
> > > >
> > > > I'm wondering what the point of defining _POSIX_THREAD_CPUTIME can be
> > > > then :-/
> > > >
> > >
> > > The pthread_getcpuclockid() function is declared in the include file
> > > /usr/include/pthread.h.  The only difference between the two "versions"
> > of
> > > Solaris 11.4 is that the newer version has a declaration and the older
> > one
> > > does not.
> > >
> > > However, _POSIX_THREAD_CPUTIME is defined in the /usr/include/unistd.h
> > file as
> > > -1 in the UNIX 03 block of options that are not supported in Solaris
> > 11.4.
> > >
> > > /* Unsupported UNIX 03 options */
> > > #if defined(_XPG6)
> > > ..
> > > #define _POSIX_THREAD_CPUTIME (-1)
> > > ..
> > > #endif
> > >
> > >
> > > An explanation of that definition can be found here:
> > >
> > > https://docs.oracle.com/cd/E88353_01/html/E37842/unistd-3head.html
> > >
> > > "If a symbolic constant is defined with the value -1, the option is not
> > > supported. Headers, data types, and function interfaces required only
> > for the
> > > option need not be supplied. An application that attempts to use anything
> > > associated only with the option is considered to be requiring an
> > extension.
> > (...)
> >
> > Ah excellent, that's quite useful! We're already doing that with
> > _POSIX_TIMERS. So I guess one just needs to try this instead:
> >
> > -#if defined(_POSIX_TIMERS) && (_POSIX_TIMERS > 0) &&
> > defined(_POSIX_THREAD_CPUTIME
> > +#if defined(_POSIX_TIMERS) && (_POSIX_TIMERS > 0) &&
> > defined(_POSIX_THREAD_CPUTIME && (_POSIX_THREAD_CPUTIME >= 0)
> >
> 
> that worked (I added closing bracket after second "defined")

Ah yes indeed. Thanks for the test. Do you want to update you patch maybe,
since you can test it ?

Thanks,
Willy

Re: [PATCH 1/3] BUILD: illumos: pthread_getcpuclockid is not available

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

вс, 5 мая 2024 г. в 10:42, Willy Tarreau :

> On Sun, May 05, 2024 at 09:12:41AM +0200, Miroslav Zagorac wrote:
> > On 05. 05. 2024. 08:32, Willy Tarreau wrote:
> > > On Sun, May 05, 2024 at 07:49:55AM +0200,  ??? wrote:
> > >> ??, 5 ??? 2024 ?. ? 02:05, Miroslav Zagorac :
> > >>> I think that this patch is not satisfactory because, for example,
> Solaris
> > >>> 11.4.0.0.1.15.0 (from 2018) has _POSIX_TIMERS and
> _POSIX_THREAD_CPUTIME
> > >>> defined, but does not have the pthread_getcpuclockid() function;
> while
> > >>> solaris
> > >>> 11.4.42.0.0.111.0 (from 2022) has that function.
> > >>>
> > >>
> > >> I'm trying to build on this vmactions/solaris-vm: Use Solaris in
> github
> > >> actions 
> > >> it does not have pthread_getcpuclockid()
> > >
> > > I'm wondering what the point of defining _POSIX_THREAD_CPUTIME can be
> > > then :-/
> > >
> >
> > The pthread_getcpuclockid() function is declared in the include file
> > /usr/include/pthread.h.  The only difference between the two "versions"
> of
> > Solaris 11.4 is that the newer version has a declaration and the older
> one
> > does not.
> >
> > However, _POSIX_THREAD_CPUTIME is defined in the /usr/include/unistd.h
> file as
> > -1 in the UNIX 03 block of options that are not supported in Solaris
> 11.4.
> >
> > /* Unsupported UNIX 03 options */
> > #if defined(_XPG6)
> > ..
> > #define _POSIX_THREAD_CPUTIME (-1)
> > ..
> > #endif
> >
> >
> > An explanation of that definition can be found here:
> >
> > https://docs.oracle.com/cd/E88353_01/html/E37842/unistd-3head.html
> >
> > "If a symbolic constant is defined with the value -1, the option is not
> > supported. Headers, data types, and function interfaces required only
> for the
> > option need not be supplied. An application that attempts to use anything
> > associated only with the option is considered to be requiring an
> extension.
> (...)
>
> Ah excellent, that's quite useful! We're already doing that with
> _POSIX_TIMERS. So I guess one just needs to try this instead:
>
> -#if defined(_POSIX_TIMERS) && (_POSIX_TIMERS > 0) &&
> defined(_POSIX_THREAD_CPUTIME
> +#if defined(_POSIX_TIMERS) && (_POSIX_TIMERS > 0) &&
> defined(_POSIX_THREAD_CPUTIME && (_POSIX_THREAD_CPUTIME >= 0)
>

that worked (I added closing bracket after second "defined")


>
> Please note that it appears at a few places, so we'll probably have to
> move that painful definition in compat.h I think.
>


that makes sense


>
> Willy
>

Re: [PATCH 1/3] BUILD: illumos: pthread_getcpuclockid is not available

[Willy Tarreau]

On Sun, May 05, 2024 at 09:12:41AM +0200, Miroslav Zagorac wrote:
> On 05. 05. 2024. 08:32, Willy Tarreau wrote:
> > On Sun, May 05, 2024 at 07:49:55AM +0200,  ??? wrote:
> >> ??, 5 ??? 2024 ?. ? 02:05, Miroslav Zagorac :
> >>> I think that this patch is not satisfactory because, for example, Solaris
> >>> 11.4.0.0.1.15.0 (from 2018) has _POSIX_TIMERS and _POSIX_THREAD_CPUTIME
> >>> defined, but does not have the pthread_getcpuclockid() function; while
> >>> solaris
> >>> 11.4.42.0.0.111.0 (from 2022) has that function.
> >>>
> >>
> >> I'm trying to build on this vmactions/solaris-vm: Use Solaris in github
> >> actions 
> >> it does not have pthread_getcpuclockid()
> > 
> > I'm wondering what the point of defining _POSIX_THREAD_CPUTIME can be
> > then :-/
> > 
> 
> The pthread_getcpuclockid() function is declared in the include file
> /usr/include/pthread.h.  The only difference between the two "versions" of
> Solaris 11.4 is that the newer version has a declaration and the older one
> does not.
> 
> However, _POSIX_THREAD_CPUTIME is defined in the /usr/include/unistd.h file as
> -1 in the UNIX 03 block of options that are not supported in Solaris 11.4.
> 
> /* Unsupported UNIX 03 options */
> #if defined(_XPG6)
> ..
> #define _POSIX_THREAD_CPUTIME (-1)
> ..
> #endif
> 
> 
> An explanation of that definition can be found here:
> 
> https://docs.oracle.com/cd/E88353_01/html/E37842/unistd-3head.html
> 
> "If a symbolic constant is defined with the value -1, the option is not
> supported. Headers, data types, and function interfaces required only for the
> option need not be supplied. An application that attempts to use anything
> associated only with the option is considered to be requiring an extension.
(...)

Ah excellent, that's quite useful! We're already doing that with
_POSIX_TIMERS. So I guess one just needs to try this instead:

-#if defined(_POSIX_TIMERS) && (_POSIX_TIMERS > 0) && 
defined(_POSIX_THREAD_CPUTIME
+#if defined(_POSIX_TIMERS) && (_POSIX_TIMERS > 0) && 
defined(_POSIX_THREAD_CPUTIME && (_POSIX_THREAD_CPUTIME >= 0)

Please note that it appears at a few places, so we'll probably have to
move that painful definition in compat.h I think.

Willy

Re: [PATCH 1/3] BUILD: illumos: pthread_getcpuclockid is not available

[Willy Tarreau]

On Sun, May 05, 2024 at 08:52:08AM +0200,  ??? wrote:
> > I'm wondering what the point of defining _POSIX_THREAD_CPUTIME can be
> > then :-/
> >
> > Just guessing, are you sure you're building with -pthread -lrt ? Just in
> > case, please double-check with V=1. Solaris sets USE_RT, but maybe
> > something
> > else is needed.
> >
> 
> I did "find / -name pthread.h -exec cat {} ';' -print"
> and there was not declaration of pthread_getcpuclockid()
> 
> chances are that it is shipped in a lib, but the prototype is missing ...

Ah I had the impression based on your error output that the definition
was there and it was only the lib that was missing. Maybe I misread
but it looked like a link error.

Willy

Re: [PATCH 1/3] BUILD: illumos: pthread_getcpuclockid is not available

[Miroslav Zagorac]

On 05. 05. 2024. 08:32, Willy Tarreau wrote:
> On Sun, May 05, 2024 at 07:49:55AM +0200,  ??? wrote:
>> ??, 5 ??? 2024 ?. ? 02:05, Miroslav Zagorac :
>>> I think that this patch is not satisfactory because, for example, Solaris
>>> 11.4.0.0.1.15.0 (from 2018) has _POSIX_TIMERS and _POSIX_THREAD_CPUTIME
>>> defined, but does not have the pthread_getcpuclockid() function; while
>>> solaris
>>> 11.4.42.0.0.111.0 (from 2022) has that function.
>>>
>>
>> I'm trying to build on this vmactions/solaris-vm: Use Solaris in github
>> actions 
>> it does not have pthread_getcpuclockid()
> 
> I'm wondering what the point of defining _POSIX_THREAD_CPUTIME can be
> then :-/
> 

The pthread_getcpuclockid() function is declared in the include file
/usr/include/pthread.h.  The only difference between the two "versions" of
Solaris 11.4 is that the newer version has a declaration and the older one
does not.

However, _POSIX_THREAD_CPUTIME is defined in the /usr/include/unistd.h file as
-1 in the UNIX 03 block of options that are not supported in Solaris 11.4.

/* Unsupported UNIX 03 options */
#if defined(_XPG6)
..
#define _POSIX_THREAD_CPUTIME (-1)
..
#endif


An explanation of that definition can be found here:

https://docs.oracle.com/cd/E88353_01/html/E37842/unistd-3head.html

"If a symbolic constant is defined with the value -1, the option is not
supported. Headers, data types, and function interfaces required only for the
option need not be supplied. An application that attempts to use anything
associated only with the option is considered to be requiring an extension.

If a symbolic constant is defined with a value greater than zero, the option
is always supported when the application is executed. All headers, data types,
and functions are present and operate as specified.

If a symbolic constant is defined with the value zero, all headers, data
types, and functions are present. The application can check at runtime to see
whether the option is supported by calling fpathconf(), pathconf(), or
sysconf() with the indicated name parameter."

-- 
Miroslav Zagorac

What can change the nature of a man?

Re: [PATCH 1/3] BUILD: illumos: pthread_getcpuclockid is not available

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

вс, 5 мая 2024 г. в 08:32, Willy Tarreau :

> On Sun, May 05, 2024 at 07:49:55AM +0200,  ??? wrote:
> > ??, 5 ??? 2024 ?. ? 02:05, Miroslav Zagorac :
> >
> > > On 04. 05. 2024. 17:36, Ilya Shipitsin wrote:
> > > > this function is considered optional for POSIX and not implemented
> > > > on Illumos
> > > >
> > > > Reference:
> > >
> https://www.gnu.org/software/gnulib/manual/html_node/pthread_005fgetcpuclockid.html
> > > > According to
> > > https://github.com/cpredef/predef/blob/master/OperatingSystems.md
> Illumos
> > > > is identified by __illumos__ macro available since gcc-11
> > > > ---
> > > >  src/clock.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/src/clock.c b/src/clock.c
> > > > index ec2133c8b..f484c2d9c 100644
> > > > --- a/src/clock.c
> > > > +++ b/src/clock.c
> > > > @@ -135,7 +135,7 @@ uint64_t now_cpu_time_thread(int thr)
> > > >  /* set the clock source for the local thread */
> > > >  void clock_set_local_source(void)
> > > >  {
> > > > -#if defined(_POSIX_TIMERS) && (_POSIX_TIMERS > 0) &&
> > > defined(_POSIX_THREAD_CPUTIME)
> > > > +#if defined(_POSIX_TIMERS) && (_POSIX_TIMERS > 0) &&
> > > defined(_POSIX_THREAD_CPUTIME) && !defined(__illumos__)
> > > >  #ifdef USE_THREAD
> > > >   pthread_getcpuclockid(pthread_self(),
> _thread_clock_id[tid]);
> > > >  #else
> > >
> > > Hello Ilya,
> > >
> > > I think that this patch is not satisfactory because, for example,
> Solaris
> > > 11.4.0.0.1.15.0 (from 2018) has _POSIX_TIMERS and _POSIX_THREAD_CPUTIME
> > > defined, but does not have the pthread_getcpuclockid() function; while
> > > solaris
> > > 11.4.42.0.0.111.0 (from 2022) has that function.
> > >
> >
> > I'm trying to build on this vmactions/solaris-vm: Use Solaris in github
> > actions 
> > it does not have pthread_getcpuclockid()
>
> I'm wondering what the point of defining _POSIX_THREAD_CPUTIME can be
> then :-/
>
> Just guessing, are you sure you're building with -pthread -lrt ? Just in
> case, please double-check with V=1. Solaris sets USE_RT, but maybe
> something
> else is needed.
>

I did "find / -name pthread.h -exec cat {} ';' -print"
and there was not declaration of pthread_getcpuclockid()

chances are that it is shipped in a lib, but the prototype is missing ...


>
> Willy
>

Re: [PATCH 1/3] BUILD: illumos: pthread_getcpuclockid is not available

[Willy Tarreau]

On Sun, May 05, 2024 at 07:49:55AM +0200,  ??? wrote:
> ??, 5 ??? 2024 ?. ? 02:05, Miroslav Zagorac :
> 
> > On 04. 05. 2024. 17:36, Ilya Shipitsin wrote:
> > > this function is considered optional for POSIX and not implemented
> > > on Illumos
> > >
> > > Reference:
> > https://www.gnu.org/software/gnulib/manual/html_node/pthread_005fgetcpuclockid.html
> > > According to
> > https://github.com/cpredef/predef/blob/master/OperatingSystems.md Illumos
> > > is identified by __illumos__ macro available since gcc-11
> > > ---
> > >  src/clock.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/src/clock.c b/src/clock.c
> > > index ec2133c8b..f484c2d9c 100644
> > > --- a/src/clock.c
> > > +++ b/src/clock.c
> > > @@ -135,7 +135,7 @@ uint64_t now_cpu_time_thread(int thr)
> > >  /* set the clock source for the local thread */
> > >  void clock_set_local_source(void)
> > >  {
> > > -#if defined(_POSIX_TIMERS) && (_POSIX_TIMERS > 0) &&
> > defined(_POSIX_THREAD_CPUTIME)
> > > +#if defined(_POSIX_TIMERS) && (_POSIX_TIMERS > 0) &&
> > defined(_POSIX_THREAD_CPUTIME) && !defined(__illumos__)
> > >  #ifdef USE_THREAD
> > >   pthread_getcpuclockid(pthread_self(), _thread_clock_id[tid]);
> > >  #else
> >
> > Hello Ilya,
> >
> > I think that this patch is not satisfactory because, for example, Solaris
> > 11.4.0.0.1.15.0 (from 2018) has _POSIX_TIMERS and _POSIX_THREAD_CPUTIME
> > defined, but does not have the pthread_getcpuclockid() function; while
> > solaris
> > 11.4.42.0.0.111.0 (from 2022) has that function.
> >
> 
> I'm trying to build on this vmactions/solaris-vm: Use Solaris in github
> actions 
> it does not have pthread_getcpuclockid()

I'm wondering what the point of defining _POSIX_THREAD_CPUTIME can be
then :-/

Just guessing, are you sure you're building with -pthread -lrt ? Just in
case, please double-check with V=1. Solaris sets USE_RT, but maybe something
else is needed.

Willy

Re: [PATCH 1/3] BUILD: illumos: pthread_getcpuclockid is not available

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

вс, 5 мая 2024 г. в 02:05, Miroslav Zagorac :

> On 04. 05. 2024. 17:36, Ilya Shipitsin wrote:
> > this function is considered optional for POSIX and not implemented
> > on Illumos
> >
> > Reference:
> https://www.gnu.org/software/gnulib/manual/html_node/pthread_005fgetcpuclockid.html
> > According to
> https://github.com/cpredef/predef/blob/master/OperatingSystems.md Illumos
> > is identified by __illumos__ macro available since gcc-11
> > ---
> >  src/clock.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/clock.c b/src/clock.c
> > index ec2133c8b..f484c2d9c 100644
> > --- a/src/clock.c
> > +++ b/src/clock.c
> > @@ -135,7 +135,7 @@ uint64_t now_cpu_time_thread(int thr)
> >  /* set the clock source for the local thread */
> >  void clock_set_local_source(void)
> >  {
> > -#if defined(_POSIX_TIMERS) && (_POSIX_TIMERS > 0) &&
> defined(_POSIX_THREAD_CPUTIME)
> > +#if defined(_POSIX_TIMERS) && (_POSIX_TIMERS > 0) &&
> defined(_POSIX_THREAD_CPUTIME) && !defined(__illumos__)
> >  #ifdef USE_THREAD
> >   pthread_getcpuclockid(pthread_self(), _thread_clock_id[tid]);
> >  #else
>
> Hello Ilya,
>
> I think that this patch is not satisfactory because, for example, Solaris
> 11.4.0.0.1.15.0 (from 2018) has _POSIX_TIMERS and _POSIX_THREAD_CPUTIME
> defined, but does not have the pthread_getcpuclockid() function; while
> solaris
> 11.4.42.0.0.111.0 (from 2022) has that function.
>

I'm trying to build on this vmactions/solaris-vm: Use Solaris in github
actions 
it does not have pthread_getcpuclockid()



>
> Maybe it could be solved in a different way..
>
>
> Best regards,
>
> --
> Miroslav Zagorac
>
> What can change the nature of a man?
>

Re: [PATCH 1/3] BUILD: illumos: pthread_getcpuclockid is not available

[Miroslav Zagorac]

On 04. 05. 2024. 17:36, Ilya Shipitsin wrote:
> this function is considered optional for POSIX and not implemented
> on Illumos
> 
> Reference: 
> https://www.gnu.org/software/gnulib/manual/html_node/pthread_005fgetcpuclockid.html
> According to 
> https://github.com/cpredef/predef/blob/master/OperatingSystems.md Illumos
> is identified by __illumos__ macro available since gcc-11
> ---
>  src/clock.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/clock.c b/src/clock.c
> index ec2133c8b..f484c2d9c 100644
> --- a/src/clock.c
> +++ b/src/clock.c
> @@ -135,7 +135,7 @@ uint64_t now_cpu_time_thread(int thr)
>  /* set the clock source for the local thread */
>  void clock_set_local_source(void)
>  {
> -#if defined(_POSIX_TIMERS) && (_POSIX_TIMERS > 0) && 
> defined(_POSIX_THREAD_CPUTIME)
> +#if defined(_POSIX_TIMERS) && (_POSIX_TIMERS > 0) && 
> defined(_POSIX_THREAD_CPUTIME) && !defined(__illumos__)
>  #ifdef USE_THREAD
>   pthread_getcpuclockid(pthread_self(), _thread_clock_id[tid]);
>  #else

Hello Ilya,

I think that this patch is not satisfactory because, for example, Solaris
11.4.0.0.1.15.0 (from 2018) has _POSIX_TIMERS and _POSIX_THREAD_CPUTIME
defined, but does not have the pthread_getcpuclockid() function; while solaris
11.4.42.0.0.111.0 (from 2022) has that function.

Maybe it could be solved in a different way..


Best regards,

-- 
Miroslav Zagorac

What can change the nature of a man?

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/