Re: [PATCH] FEATURE/MAJOR: Add upstream-proxy-tunnel feature

[Christopher Faulet]

Le 19/09/2024 à 13:04, Aleksandar Lazic a écrit :
> Hi Willy.
>
> any chance to look into the Answer?
>

Hi Alex,

I've started the review. First of all I agree with Willy. I guess it is not what
many people have in mind when they are talking about supporting configuration of
an HTTP proxy to connect to a server. But, by explaining it as a "sock4" like
mode, it makes sense. So we must just take care to be clear on this point to
avoid any confusion. And be sure it will still be possible to implement the
other mode without conflict. At first glance, it seems good this way. We must
find the right name for the server line keyword. But it is not a big deal. We
can work on the implementation in meantime.

On the form, I know it is still a work in-progress, but a big patch is still
harder to read than a series of smaller modifications. In addition, there are
still plenty of DPRINTF that make the review more difficult. And some parts of
the code are commented and must not be included in the patch, still for
readability. If this concerns something you want to implement but you need some
help to understand how to do it, it is better to add a comment to explain what
you want to achieve. It is more understandable for the reviewer and it is a
better way to expose your idea.

I suggest to split your patch this way:

  * .If some existing fields must be renamed to support this feature, this may
be done in dedicated patches.

  * Then, a patch must contain the basic feature and its documentation. So the
server line keyword to configure the upstream proxy and the handshake part. It
is the main part of the feature

  * The rules to customize the feature must be move in another patch.

  * Another one with additional ressources, if any (exemples, diagrams...)

  * A final patch with a reg-test if possible. I don't know if the feature can
be tested with VTEST. I guess it is possible to do basic tests.

  * If some traces must be added on handshakes, it must be done in a dedicated
patch.

Of course you can ask some help. It is not trivial. But the idea is to cut as
far as possible the feature to smaller and consistant part. If it is too huge
and it cannot be split, there is probably (but not necessarily) an issue with
the feature.

Now, on the implementation itself. First, from my point of view, the feature
should work without any rules. So, setting the right keyword on the server line
should be good enough to make it works. If the server has a hostname, it should
be used indeed. But otherwise its address must be used. There is not reason to
make it fails in that case. I guess "sock4_addr" and
"upstream_proxy_tunnel_addr", in the server structure, can be merged into one
field. It seems reasonable to make these both handshakes mutually-exclusive. An
error can even be triggered during the configuration parsing if both keyword are
used on the same server line.

You must also take care to send a valid request, with a Host header. Its value
must be set accordingly to the authority. The response parsing must also be
reviewed to handle any 2xx response and to read all headers. I guess that for
now, we can consume everything till the end of headers (2 x CRLF). Otherwise,
any headers will be considered as part of the response to the client. Here at
most one buffer of data must be allowed and you must have the same construct
than the other handshakes: peeking data waiting for a full response and really
consuming them at the end.

The actions parsing must not be inline. A dedicated parsing function must be
used. And there is no need to add new actions in "act_name" enum. You must set
it to ACT_CUSTOM and use dedicated functions. Arguments for the action must be
stored in "act_rule.arg" union. "act_rule.arg.http may be used or eventually
"act_rule.arg.act". A more annoying point is that you must not see these actions
as part of a dedicated ruleset. These actions must be evaluated exactly where
there are defined, respecting the rules order. It is the expected behavior.
Especially if log-format string or expression are used. It is not implemented
yet but it is expected to be supported. In that case the following may be 
written:

   tcp-request content set-var(txn.dummy) int(1)
   tcp-request content upstream-proxy-header My-header "%[var(txn.dummy)]"
   tcp-request content set-var(txn.dummy) int(2)

Here "My-header: 1" must be sent to the proxy. I guess the "updt_rules" list and
the buffers must be removed from the proxy structure. The result of the rules
evaluation must however be stored somewhere in the stream. Here we must think a
bit about how to do so. It may be a dedicated structure with a target name and a
list of headers. For me, if we do so, this struct must be allocated on demand.
There is no reason to use too many memory for a marginal feature. It can even be
an HTX buffer. Some functions are probably missing in the current HTX api. But
it may be handy. Especially if we consider to add more actions in futures. And
it could b

Re: haproxy-3.x.x - Ubuntu Focal

[Vincent Bernat]

On 2024-09-23 12:34, Alexis Vachette wrote:

It seems that version 3.0.5-1 build is broken because of systemd-dev 
package missing.


Inspecting the log file I do see mention of several others packages:

The following packages have unmet dependencies: sbuild-build-depends- 
main-dummy :

Depends: liblua5.4-dev but it is not installable
Depends: libopentracing-c-wrapper-dev but it is not installable
Depends: systemd-dev but it is not installable


I have pushed a new version. The problem was also present in 2.9 branch.

Re: haproxy-3.x.x - Ubuntu Focal

[William Lallemand]

On Mon, Sep 23, 2024 at 01:33:27PM +0200, William Lallemand wrote:
> On Mon, Sep 23, 2024 at 12:34:54PM +0200, Alexis Vachette wrote:
> > Hi Vincent,
> > 
> > Thank you.
> > 
> > It seems that version 3.0.5-1 build is broken because of systemd-dev
> > package missing.
> > 
> > Inspecting the log file I do see mention of several others packages:
> > 
> > The following packages have unmet dependencies:
> > sbuild-build-depends-main-dummy :
> > Depends: liblua5.4-dev but it is not installable
> > Depends: libopentracing-c-wrapper-dev but it is not installable
> > Depends: systemd-dev but it is not installable
> > 
> > Regards,
> > 
> > On Sat, 14 Sept 2024 at 11:03, Vincent Bernat  wrote:
> > 
> > > On 2024-09-11 08:40, Alexis Vachette wrote:
> > >
> > > > Just wanted to know if you had a plan to release package for Ubuntu
> > > > 20.04 Focal.
> > > >
> > > > Mostly because of OpenSSL 3.0 regression performance.
> > > >
> > > > The question is more for Vincent Bernat.
> > >
> > > I've just pushed a build for Ubuntu Focal.
> > >
> 
> Since 3.0 libsystemd is not a dependance anymore, this could be removed 
> completely,
> it was only used for sd_notify(), but since the openssh/xz incident we 
> reimplemented
> the function to get rid of the lib.
> 

My mistake, systemd-dev and libsystem-dev are not the same thing.

-- 
William Lallemand

Re: haproxy-3.x.x - Ubuntu Focal

[William Lallemand]

Hello,

On Mon, Sep 23, 2024 at 12:34:54PM +0200, Alexis Vachette wrote:
> Hi Vincent,
> 
> Thank you.
> 
> It seems that version 3.0.5-1 build is broken because of systemd-dev
> package missing.
> 
> Inspecting the log file I do see mention of several others packages:
> 
> The following packages have unmet dependencies:
> sbuild-build-depends-main-dummy :
> Depends: liblua5.4-dev but it is not installable
> Depends: libopentracing-c-wrapper-dev but it is not installable
> Depends: systemd-dev but it is not installable
> 
> Regards,
> 
> On Sat, 14 Sept 2024 at 11:03, Vincent Bernat  wrote:
> 
> > On 2024-09-11 08:40, Alexis Vachette wrote:
> >
> > > Just wanted to know if you had a plan to release package for Ubuntu
> > > 20.04 Focal.
> > >
> > > Mostly because of OpenSSL 3.0 regression performance.
> > >
> > > The question is more for Vincent Bernat.
> >
> > I've just pushed a build for Ubuntu Focal.
> >

Since 3.0 libsystemd is not a dependance anymore, this could be removed 
completely,
it was only used for sd_notify(), but since the openssh/xz incident we 
reimplemented
the function to get rid of the lib.

-- 
William Lallemand

Re: haproxy-3.x.x - Ubuntu Focal

[Alexis Vachette]

Hi Vincent,

Thank you.

It seems that version 3.0.5-1 build is broken because of systemd-dev
package missing.

Inspecting the log file I do see mention of several others packages:

The following packages have unmet dependencies:
sbuild-build-depends-main-dummy :
Depends: liblua5.4-dev but it is not installable
Depends: libopentracing-c-wrapper-dev but it is not installable
Depends: systemd-dev but it is not installable

Regards,

On Sat, 14 Sept 2024 at 11:03, Vincent Bernat  wrote:

> On 2024-09-11 08:40, Alexis Vachette wrote:
>
> > Just wanted to know if you had a plan to release package for Ubuntu
> > 20.04 Focal.
> >
> > Mostly because of OpenSSL 3.0 regression performance.
> >
> > The question is more for Vincent Bernat.
>
> I've just pushed a build for Ubuntu Focal.
>


-- 


*Alexis Vachette*Expert Network Engineer

Re: [PATCH] FEATURE/MAJOR: Add upstream-proxy-tunnel feature

[Aleksandar Lazic]

Hi Christopher.

On 2024-09-19 (Do.) 16:00, Christopher Faulet wrote:

Le 19/09/2024 à 13:04, Aleksandar Lazic a écrit :

Hi Willy.

any chance to look into the Answer?



Hi Alex,

In fact, Willy asked me to take a look on this feature. I started to work on it 
and I was distracted by the stable releases and also by some tricky bugs. I'm 
going to start again to review your patchs. I guess I will be able to give you a 
feedback soon. Very sorry for the delay !


Thank you for your Answer.

No Problem, it is a long term requested feature so we are not in any rush here, 
take your time :-)



Regards,


Regards
Alex

Re: [PATCH] FEATURE/MAJOR: Add upstream-proxy-tunnel feature

[Christopher Faulet]

Le 19/09/2024 à 13:04, Aleksandar Lazic a écrit :

Hi Willy.

any chance to look into the Answer?



Hi Alex,

In fact, Willy asked me to take a look on this feature. I started to work on it 
and I was distracted by the stable releases and also by some tricky bugs. I'm 
going to start again to review your patchs. I guess I will be able to give you a 
feedback soon. Very sorry for the delay !


Regards,
--
Christopher Faulet

Re: [PATCH] FEATURE/MAJOR: Add upstream-proxy-tunnel feature

[Aleksandar Lazic]

Hi Willy.

any chance to look into the Answer?

Regards

Alex

On 2024-08-15 (Do.) 10:55, Aleksandar Lazic wrote:

Hi Willy.

On 2024-08-12 (Mo.) 16:49, Willy Tarreau wrote:

Hi Alex,

On Mon, Aug 12, 2024 at 11:46:37AM +0200, Aleksandar Lazic wrote:

On Thu, Jun 13, 2024 at 03:00:59AM +0200, Aleksandar Lazic wrote:

The final idea is something like this.

```
tcp-request content upstream-proxy-header Host %[req.ssl_sni,lower]
tcp-request content upstream-proxy-header "$AUTH" "$TOKEN"
tcp-request content upstream-proxy-header Proxy-Connection Keep-Alive
tcp-request content upstream-proxy-target %[req.ssl_sni,lower]

server stream 0.0.0.0:0 upstream-proxy-tunnel
%[req.ssl_sni,lower,map_str(targets.map)]
```

The targets.map should have something like this.
#dest proxy
sni01 proxy01
sni02 proxy02

I hope the background of upstream-proxy-target is now more clear.

- In `server https_Via_Proxy1 0.0.0.0:0 upstream-proxy-tunnel 
127.0.0.1:3128`
     from your config, what is 0.0.0.0:0 used for here? This binds to 
all IPv4

     but on a random free port?


This is required when the destination should be dynamic, it's documented 
here.

http://docs.haproxy.org/3.0/configuration.html#4.4-do-resolve


A+
Dave


Regards
Alex


Overall I find that there are plenty of good points in this patch and
the discussion around it. But it also raises questions:

    - IMHO the server's address should definitely be the proxy's address.
  This will permit to use all the standard mechanisms we have such as
  DNS resolution and health checks and continues to work as a regular
  handshake. I'm not sure this is the case in the current state of the
  patch since I'm seeing an extra "upstream-proxy-tunnel" parameter
  (actually I'm confused by what you call a "target" here, as used in
  upstream-proxy-target).


Well I thought the same but that's then difficult to separate between the
proxy server and the destination server. Maybe we need some kind of server
type?

As you can see in the sequence diagram are both servers from HAProxy point 
of view.

https://github.com/git001/tls-proxy-tunnel/blob/main/README.md#sequence-diagram

The "target" is the "destination server" which is from my point of view the
"server" in haproxy.

The "upstream-proxy-target" is the upstream proxy server. I'm also not happy
with that name and open for suggestions.


Then I'm confused by what the server's configured address becomes.


Fully understand.
Let me separate the word "server" here.

The server in the server line is the final target/destination server which is 
the one which the user want to reach.


The upstream proxy server, similar to the socks4 server, is just a hop server 
to connect to the final target/destination server.



I mean, there are three possibilities:

   1) the ip:port of the server designates the target server, in which
  case only ip:port are passed to the proxy and we never send an FQDN
  there. Then the proxy has to be hard-coded in an option (apparently
  the upstream-proxy-tunnel here). Haproxy then focuses only on the
  target server for load balancing, stickiness, health checks etc and
  always reaches that server through the hard-coded upstream proxy.
  This could match what an application-oriente LB would look for, i.e.
  being able to reach its "local" servers through a proxy. It's just
  that I would be surprised that such a use case really exists, because
  load-balancing remote servers across a single local hard-coded proxy
  doesn't feel natural at all. E.g. the LB is not even on the LAN that
  knows all the remote hosts so such a config is particularly difficult
  to maintain in a working state.

   2) the fqdn:port of the server designates the target server, in which
  case only fqdn:port are passed to the proxy and we let the proxy
  resolve the target server. The rest is the same as above, it's just
  a small variation, but I'm still having a hard time figuring what
  use case that could match.


This is the current implementation of the patch.
The Proxy is the one which then decides if the connection to the 
target/destination server is allowed or not.



   3) the ip:port of the server designates the upstream proxy, and the
  connection's destination is passed to the CONNECT method. In this
  case it means that LB, checks, stickiness etc applies to the proxies
  and not to the servers. In this case we need a distinct field to set
  the target host name. That's more or less what you did with the
  tcp-request upstream-proxy-target rule, which could possible have a
  default on the server line. We could also imagine that a set-dst
  action would allow to force the target destination address, but that
  would be a detail. This more is more about infrastructure than remote
  app servers: you tell the proxy how to reach external services. I've
  seen this requested a few times by those sa

Re: [PATCH] DOC: management: add init-state to add server keywords

[Willy Tarreau]

On Tue, Sep 17, 2024 at 02:54:24PM +, Damien Claisse wrote:
> Commit ce6a621ae allowed init-state to be used for dynamic servers but I
> forgot to update management doc.

Applied, thank you Damien!
Willy

Re: Website Enquiry (Outreach Department)

[Henry Pan (HP)]

ok just $1k

Thanks & Best Regards

Henry PAN
Sr. Lead Cloud Architect
(425) 802--3975
https://www.linkedin.com/in/henrypan1



On Mon, Sep 16, 2024 at 2:27 AM Patricia Tolson 
wrote:

> Hi,
>
> This is Patricia from Outreach Department,
>
> I would like to ask how much would you charge for posting an informative
> non-promotional article with do-follow links on haproxy.com?
>
> Before we could start, I just have a few clarifications that I need to
> ask: *PLEASE ANSWER ALL.*
>
> 1st, Please make sure our articles along with do-follow links will be
> posted permanently for as long as your website stays active and be covered
> under a suitable category;
>
> 2nd, It should not be marked as sponsored anywhere nor shall it say guest
> post or guest contributor on the page. We also need our links to have no
> link attributes such as Rel=" sponsored" ;
>
> 3rd, Please make sure our article will not be posted on a subdomain. All
> of our customers will be looking at your main site rather than your
> sub-domain if you have one;
>
> 4th, Please make sure you have no indexing issues with your live links.
> Please don't worry if an indexing issue happens because we have a team who
> can walk you through on how to resolve it;
>
> 5th, What is the turnaround time for you to publish our article once you
> receive it?
>
> 6th, Is your site WordPress? If yes, would you consider setting up a
> contributor account?
>
> 7th, We would also like to know if you are accepting
> - articles/links about gambling and cannabis.
> - Link insertion to an existing article.
> This is for our reference. If you do, please include the price.
>
> 8th, Is it okay if we contact you during weekends? I know how busy you are
> so we'd like to know ahead of time.
>
> 9th, Please provide us your PayPal account. Regarding payment, please
> invoice us via PayPal? If not, you may send us a pdf invoice. Our payment
> process takes 5-7 days after you send us the live link. We are system
> generated and payments are made every once a week.
>
> 10th & Last, please provide your post/article guidelines for us, e.g.,
> word count, etc. And additional anchor guidelines if you have some.
>
> And if ever you have any other contact information that is more convenient
> for you like another email address, Skype, Facebook messenger or Phone
> number, we'd appreciate it.
>
> Looking forward to hearing from you.
>
>
> *Regards,Patricia*
>

Re: haproxy-3.x.x - Ubuntu Focal

[Vincent Bernat]

On 2024-09-11 08:40, Alexis Vachette wrote:

Just wanted to know if you had a plan to release package for Ubuntu 
20.04 Focal.


Mostly because of OpenSSL 3.0 regression performance.

The question is more for Vincent Bernat.


I've just pushed a build for Ubuntu Focal.

Re: haproxy-3.x.x - Ubuntu Focal

[Willy Tarreau]

On Wed, Sep 11, 2024 at 10:14:13AM +0200,  ??? wrote:
> ??, 11 . 2024 ?. ? 08:44, Alexis Vachette :
> 
> > Hi,
> >
> > Just wanted to know if you had a plan to release package for Ubuntu 20.04
> > Focal.
> >
> > Mostly because of OpenSSL 3.0 regression performance.
> >
> > The question is more for Vincent Bernat.
> >
> 
> I wonder what are your expectation of SSL lib for that package. I do not
> see good choice
> 
> 1) OpenSSL-1.1.1 (only limited QUIC unfortunately)
> 2) OpenSSL-3.X (bad perf)
> 3) QuicTLS (development frozen)
> 4) AWS-LC, WolfSSL, LibreSSL (requires efforts from packaging)
> 
> or, if you do not plan to use QUIC, OpenSSL-1.1.1 would b just nice

FWIW, at HaproxyTech, we decided to stick to OpenSSL-1.1.1 for now
and are providing packages built with it. We've already got several
reports of outages with 3.0 (not surprising) and are systematically
directing customers to the 1.1.1 packages to avoid any future problem.

I've read on the Ubuntu blog that they're going to maintain their 1.1.1
package up to 2030. I don't know if it's possible to install a package
of an older distro on a newer one, but that could be convenient.

Otherwise if you only want packaged stuff for Ubuntu 20, Vincent still
provides haproxy up to 2.9 (that includes 2.8 which is LTS). But at some
point you might have to build packages yourself if you need an extended
support on an older distro, or to update to a newer distro with a
different lib or version. It's sad but that the result of OpenSSL having
irresponsibly tagged 3.0 LTS before even testing it... It ended up in
distros while being totally unfit for a server, and distros can't easily
switch to an unsupported version, as security issues are even worse for
them than performance and stability issues.

Willy

Re: haproxy-3.x.x - Ubuntu Focal

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

ср, 11 сент. 2024 г. в 08:44, Alexis Vachette :

> Hi,
>
> Just wanted to know if you had a plan to release package for Ubuntu 20.04
> Focal.
>
> Mostly because of OpenSSL 3.0 regression performance.
>
> The question is more for Vincent Bernat.
>

I wonder what are your expectation of SSL lib for that package. I do not
see good choice

1) OpenSSL-1.1.1 (only limited QUIC unfortunately)
2) OpenSSL-3.X (bad perf)
3) QuicTLS (development frozen)
4) AWS-LC, WolfSSL, LibreSSL (requires efforts from packaging)

or, if you do not plan to use QUIC, OpenSSL-1.1.1 would b just nice


>
> Regards,
> --
>
>
> *Alexis Vachette*Expert Network Engineer
>
> 
>

Re: [PATCH] MINOR: server: allow init-state for dynamic servers

[Willy Tarreau]

On Tue, Sep 10, 2024 at 02:52:42PM +, Damien Claisse wrote:
> Commit 50322df introduced the init-state keyword, but it didn't enable
> it for dynamic servers. However, this feature is perfectly desirable
> for virtual servers too, where someone would like a server inlived
> through "set server be1/srv1 state ready" to be put out of maintenance
> in down state until the next health check succeeds.
> At reading the code, it seems that it's only a matter of allowing this
> keyword for dynamic servers, as current code path calls
> srv_adm_set_ready() which incidentally triggers a call to
> _srv_update_status_adm().

Merged, many thanks Damien, and sorry for not thinking more often about
checking dynamic servers support during reviews. It will come over time,
I just need to change 20 years-long habits :-)

Willy

Re: Website Enquiry (Outreach Department)

[Henry Pan (HP)]

Call me to answering your questions


Tks Henry Pan


Patricia Tolson 于2024年9月10日 周二上午4:14写道：

> Hi,
>
> This is Patricia from Outreach Department,
>
> I would like to ask how much would you charge for posting an informative
> non-promotional article with do-follow links on haproxy.com?
>
> Before we could start, I just have a few clarifications that I need to
> ask:
> *PLEASE ANSWER ALL.*
> 1st, Please make sure our articles along with do-follow links will be
> posted permanently for as long as your website stays active and be covered
> under a suitable category;
>
> 2nd, It should not be marked as sponsored anywhere nor shall it say guest
> post or guest contributor on the page. We also need our links to have no
> link attributes such as Rel=" sponsored" ;
>
> 3rd, Please make sure our article will not be posted on a subdomain. All
> of our customers will be looking at your main site rather than your
> sub-domain if you have one;
>
> 4th, Please make sure you have no indexing issues with your live links.
> Please don't worry if an indexing issue happens because we have a team who
> can walk you through on how to resolve it;
>
> 5th, What is the turnaround time for you to publish our article once you
> receive it?
>
> 6th, Is your site WordPress? If yes, would you consider setting up a
> contributor account?
>
> 7th, We would also like to know if you are accepting
> - articles/links about gambling and cannabis.
> - Link insertion to an existing article.
> This is for our reference. If you do, please include the price.
>
> 8th, Is it okay if we contact you during weekends? I know how busy you are
> so we'd like to know ahead of time.
>
> 9th, Please provide us your PayPal account. Regarding payment, please
> invoice us via PayPal? If not, you may send us a pdf invoice. Our payment
> process takes 5-7 days after you send us the live link. We are system
> generated and payments are made every once a week.
>
> 10th & Last, please provide your post/article guidelines for us, e.g.,
> word count, etc. And additional anchor guidelines if you have some.
>
> And if ever you have any other contact information that is more convenient
> for you like another email address, Skype, Facebook messenger or Phone
> number, we'd appreciate it.
>
> Looking forward to hearing from you.
>
>
> *Regards,Patricia*
>

Re: [PR] BUG fix in stream.c where counters will zero because of failed updates

[Willy Tarreau]

Hello!

On Fri, Sep 06, 2024 at 11:23:03AM +, PR Bot wrote:
> From d9d994eb1c6615b1f7ce7a0493be669ad8bb4ab0 Mon Sep 17 00:00:00 2001
> From: shakedm 
> Date: Fri, 6 Sep 2024 13:38:25 +0300
> Subject: [PATCH] fix a BUG in stream.c where counters will zero because of
>  failed updates
> 
> The current code only checked if bytes is initialized but during monitoring I
> found many instances in which the metrics will randomly drop to zero.
> this fix handles that scenario. I tested it on compiling and running the new
> binary and seems like it fixed the problem.

I disagree, it tests if bytes is non-zero, which is basically what you're
doing as well, look below:

> @@ -792,7 +792,7 @@ void stream_process_counters(struct stream *s)
>  
>   bytes = s->req.total - s->logs.bytes_in;
>   s->logs.bytes_in = s->req.total;
> - if (bytes) {
> + if (((signed long long) bytes) > 0) {
>   _HA_ATOMIC_ADD(&sess->fe->fe_counters.bytes_in, bytes);
>   _HA_ATOMIC_ADD(&s->be->be_counters.bytes_in,bytes);

The code checks if s->req.total changed since last time it was copied
into s->logs.bytes_in, and copies it there each time.

Your change istesting that it changed positively, but given that these
are 64-bit numbers, it's practically impossible to get negative numbers
here in one call. And I would even argue that the current test is better
since it could actually detect any change (even those large impractical
ones). So there's no problem.

You said you found instances where metrics will randomly drop to zero.
Did you find them when reading the code, or did you observe them yourself?
If the latter, then you've observed something else. The vast majority of
us are already monitoring stats, so if you've encountered a case where
such a problem happens, we'll need your config, version, and to know all
the specificities of your environment to figure how this could happen at
all.

Just thinking loud, are you sure that you're not experiencing reloads?
A reload starts a new process, and stats will restart from zero. In 3.1
there is a new option to restart the process from the current stats
counters, you might be interested in using this if that's what you're
facing.

Thanks!
Willy

Re: [PATCH v2] MEDIUM: server: add init-state

[Willy Tarreau]

On Wed, Sep 04, 2024 at 06:27:35PM -0400, Aaron Kuehler wrote:
> Allow the user to set the "initial state" of a server.
> 
> Context:
> 
> Servers are always set in an UP status by default. In
> some cases, further checks are required to determine if the server is
> ready to receive client traffic.
> 
> This introduces the "init-state {up|down}" configuration parameter to
> the server.
(...)

The patch looks good, the doc looks good and the commit message is clear
enough, that's fine, I've merged it. I'm going to mark the issue as fixed
now.

Many thanks!
Willy

Re: [PATCH] MEDIUM: server: add init-state

[Pavlos Parissis]

On Thu, 5 Sept 2024 at 05:18, Willy Tarreau  wrote:
>

[...snip...]

> > > The server's init-state is considered when the HAProxy instance
> > > is (re)started, a new server is detected (for example via service
> > > discovery / DNS resolution), a server exits maintenance, etc.
> >
> > Will this honour the functionality coming from load-server-state-from-file?
> > I assume if users utilize the aforementioned setting this new functionality,
> > which is great and thanks a lot for delivering it, will not kick in.
>
> Logiclaly it should not change anything there since the state file already
> contains the last state (srv_op_state), so the server whose state is loaded
> from the file will be restored to its previous state. What is changed here
> is the ability to set the server up/down when leaving maintenance. E.g. you
> had put a server in maintenance state, you reinstalled it, then you enable
> it from the CLI. Before it would instantly take traffic and perform a check
> to possibly turn it off again, now you'll have the option to let it run its
> checks before being used.
>
> Hoping this helps,
> Willy

Yes, it does. Thanks for the clarification.

Cheers,
Pavlos

Re: [PATCH] MEDIUM: server: add init-state

[Willy Tarreau]

Hi Pavlos,

On Wed, Sep 04, 2024 at 10:27:29AM +0200, Pavlos Parissis wrote:
> On Tue, 3 Sept 2024 at 22:26, Aaron Kuehler  
> wrote:
> >
> > Allow the user to set the "initial state" of a server.
> >
> > Context:
> >
> > Servers are always set in an UP status by default. In
> > some cases, further checks are required to determine if the server is
> > ready to receive client traffic.
> >
> > This introduces the "init-state {up|down}" configuration parameter to
> > the server.
> >
> > - when not set, the server is considered available as soon as a connection
> >   can be established.
> > - when set to 'up', the server is considered available as soon as a
> >   connection can be established.
> > - when set to 'down', the server is initially considered unavailable until
> >   it successfully completes its health checks.
> >
> > The server's init-state is considered when the HAProxy instance
> > is (re)started, a new server is detected (for example via service
> > discovery / DNS resolution), a server exits maintenance, etc.
> 
> Will this honour the functionality coming from load-server-state-from-file?
> I assume if users utilize the aforementioned setting this new functionality,
> which is great and thanks a lot for delivering it, will not kick in.

Logiclaly it should not change anything there since the state file already
contains the last state (srv_op_state), so the server whose state is loaded
from the file will be restored to its previous state. What is changed here
is the ability to set the server up/down when leaving maintenance. E.g. you
had put a server in maintenance state, you reinstalled it, then you enable
it from the CLI. Before it would instantly take traffic and perform a check
to possibly turn it off again, now you'll have the option to let it run its
checks before being used.

Hoping this helps,
Willy

Re: [PATCH] MEDIUM: server: add init-state

[Aaron Kuehler]

Greetings Willy,

> On Sep 3, 2024, at 11:48 PM, Willy Tarreau  wrote:
> 
> Hello Aaron,
> 
> On Tue, Sep 03, 2024 at 04:24:57PM -0400, Aaron Kuehler wrote:
>> Allow the user to set the "initial state" of a server.
> 
> Thank you very much for working on this one!

Thank you for the swift and clueful feedback, and kind words. Happy to have a 
go. I’m a bit out of my element, and grateful for the support. 

> 
>> Context:
>> 
>> Servers are always set in an UP status by default. In
>> some cases, further checks are required to determine if the server is
>> ready to receive client traffic.
>> 
>> This introduces the "init-state {up|down}" configuration parameter to
>> the server.
>> 
>> - when not set, the server is considered available as soon as a connection
>>  can be established.
>> - when set to 'up', the server is considered available as soon as a
>>  connection can be established.
>> - when set to 'down', the server is initially considered unavailable until
>>  it successfully completes its health checks.
> 
> I'm having a difficulty understanding the first two cases, and it seems
> from the patch that they are in fact the same. I think that "as soon as
> a connection can be established" is confusing as it makes one think that
> it still needs to perform one check (otherwise it's not clear what this
> "connection" refers to).
> 
> Thus I suggest that the first two lines are merged into one with an
> explanation like this one:
> 
>  - when set to 'up' (the default), the server is considered immediately
>available and will initiate a health check that can turn it to the DOWN
>state immediately if it fails.
> 
> And the same could be done in the doc, which contains the same text.
> 

You’re 100% correct, the first two cases have the same behavior, and I agree my 
initial wording is confusing. I like the direction in which your suggestion is 
pointed and I’ll incorporate it into the commit message and documentation. 

>> The server's init-state is considered when the HAProxy instance
>> is (re)started, a new server is detected (for example via service
>> discovery / DNS resolution), a server exits maintenance, etc.
> 
> You're right, it's important to mention this because it's far from being
> obvious.

Great point. I’ll also include this in the user-facing, configuration 
documentation. 

> 
> By analogy with the "up" state, the "down" should be at check_rise-1 
> instead of zero I think, so that it's sufficient to succeed one check
> to turn it on. Otherwise it can take a lot of time to introduce some
> servers which are slowly checked.
> 
> I'm just thinking about something, since I've read that complaint as well
> a few times: in some environments, some servers are having difficulties
> responding positively to initial health checks, and due to the way we're
> starting at the check_rise value, the first failure is sufficient to turn
> it down. A few users already asked for a way to turn the server completely
> up. I think there could be value in having 3 different init states:
>  - 'down': needs to validate one check before turning up
>  - 'probe': the current situation, where it's up until the first test
>makes it fail
>  - 'up': it starts fully up (check_rise+check_fall-1 IIRC).
> 
> Or maybe even 4 states:
>  - "fully-down": 0, requires that all checks are valid before it turns up
>  - "down": check_rise-1, requires one successful check to turn up
>  - "up": check_rise, requires one faulty check to turn down
>  - "fully-up": check_rise+check_fall-1, requires that all checks fail to
>turn down.
> 
> And we'd default to "up" like in your patch.
> 
> What do you think ?

I’m no expert, but the latter sounds slightly more flexible. I’ll make this 
change. 

> 
> BTW your patch is super clean, I've only found one tiny thing (just to
> say that I found something):
> 
>> @@ -6504,7 +6526,11 @@ static int _srv_update_status_adm(struct server *s, 
>> enum srv_adm_st_chg_cause ca
>>   */
>>  if (s->check.state & CHK_ST_ENABLED) {
>>  s->check.state &= ~CHK_ST_PAUSED;
>> -s->check.health = s->check.rise; /* start OK but check 
>> immediately */
>> +if(s->init_state == SRV_INIT_STATE_DOWN) {
> ^^
> missing space here. :-)
> 
>> +s->check.health = 0; /* checks must pass before 
>> the server is considered UP */
>> +} else {
>> +s->check.health = s->check.rise; /* start OK 
>> but check immediately */
>> +}
>>  }
> 

Thank you for the kind words, and keen eyes. I’ll fix this, promptly.

Cheers, 
Aaron

Re: [ANNOUNCE] haproxy-3.0.4

[Willy Tarreau]

Hi,

On Tue, Sep 03, 2024 at 03:52:43PM +0200, Willy Tarreau wrote:
> HAProxy 3.0.4 was released on 2024/09/03. It added 42 new commits
> after version 3.0.3.
(...)
> Note that at this point this flushes the queue of pending bugs for 3.0,
> which is a good news. There remains one exception, a recently introduced
> QUIC patchset into 3.1 to implement NEW_TOKEN on 0-RTT that we'd like to
> backport since it addresses some bad corner cases. But the backport is
> non-trivial and the patches need to be exposed a bit longer in 3.1 first,
> this might come in 3.0.5.

I shouldn't have claimed that the queue was flushed, because in the
middle of the patches marked "backport after an observation period",
Christopher found a small cluster that could have been backported as
well with this release. Nothing critical, that doesn't deserve a new
release, but we'll probably make another one once the QUIC backports
above are done, in order to *really* flush the pipe this time. It will
likely be at the same time as the next 2.8 and lower then.

Sorry about that, it's always difficult to properly spot what's eligible
yet missing (we've improved the tooling again to help detect such misses
next time). All of these have been queued into the 3.0-maint and
2.9-maint branches.

In short, no worries, but don't be surprised if you were waiting for
3.0.4 regarding a specific fix and only find it queued for next release.

Willy

Re: [PATCH] MEDIUM: server: add init-state

[Pavlos Parissis]

On Tue, 3 Sept 2024 at 22:26, Aaron Kuehler  wrote:
>
> Allow the user to set the "initial state" of a server.
>
> Context:
>
> Servers are always set in an UP status by default. In
> some cases, further checks are required to determine if the server is
> ready to receive client traffic.
>
> This introduces the "init-state {up|down}" configuration parameter to
> the server.
>
> - when not set, the server is considered available as soon as a connection
>   can be established.
> - when set to 'up', the server is considered available as soon as a
>   connection can be established.
> - when set to 'down', the server is initially considered unavailable until
>   it successfully completes its health checks.
>
> The server's init-state is considered when the HAProxy instance
> is (re)started, a new server is detected (for example via service
> discovery / DNS resolution), a server exits maintenance, etc.

Will this honour the functionality coming from load-server-state-from-file?
I assume if users utilize the aforementioned setting this new functionality,
which is great and thanks a lot for delivering it, will not kick in.

Regards,
Pavlos

Re: [PATCH] MEDIUM: server: add init-state

[Willy Tarreau]

Hello Aaron,

On Tue, Sep 03, 2024 at 04:24:57PM -0400, Aaron Kuehler wrote:
> Allow the user to set the "initial state" of a server.

Thank you very much for working on this one!

> Context:
> 
> Servers are always set in an UP status by default. In
> some cases, further checks are required to determine if the server is
> ready to receive client traffic.
> 
> This introduces the "init-state {up|down}" configuration parameter to
> the server.
> 
> - when not set, the server is considered available as soon as a connection
>   can be established.
> - when set to 'up', the server is considered available as soon as a
>   connection can be established.
> - when set to 'down', the server is initially considered unavailable until
>   it successfully completes its health checks.

I'm having a difficulty understanding the first two cases, and it seems
from the patch that they are in fact the same. I think that "as soon as
a connection can be established" is confusing as it makes one think that
it still needs to perform one check (otherwise it's not clear what this
"connection" refers to).

Thus I suggest that the first two lines are merged into one with an
explanation like this one:

  - when set to 'up' (the default), the server is considered immediately
available and will initiate a health check that can turn it to the DOWN
state immediately if it fails.

And the same could be done in the doc, which contains the same text.

> The server's init-state is considered when the HAProxy instance
> is (re)started, a new server is detected (for example via service
> discovery / DNS resolution), a server exits maintenance, etc.

You're right, it's important to mention this because it's far from being
obvious.

By analogy with the "up" state, the "down" should be at check_rise-1 
instead of zero I think, so that it's sufficient to succeed one check
to turn it on. Otherwise it can take a lot of time to introduce some
servers which are slowly checked.

I'm just thinking about something, since I've read that complaint as well
a few times: in some environments, some servers are having difficulties
responding positively to initial health checks, and due to the way we're
starting at the check_rise value, the first failure is sufficient to turn
it down. A few users already asked for a way to turn the server completely
up. I think there could be value in having 3 different init states:
  - 'down': needs to validate one check before turning up
  - 'probe': the current situation, where it's up until the first test
makes it fail
  - 'up': it starts fully up (check_rise+check_fall-1 IIRC).

Or maybe even 4 states:
  - "fully-down": 0, requires that all checks are valid before it turns up
  - "down": check_rise-1, requires one successful check to turn up
  - "up": check_rise, requires one faulty check to turn down
  - "fully-up": check_rise+check_fall-1, requires that all checks fail to
turn down.

And we'd default to "up" like in your patch.

What do you think ?

BTW your patch is super clean, I've only found one tiny thing (just to
say that I found something):

> @@ -6504,7 +6526,11 @@ static int _srv_update_status_adm(struct server *s, 
> enum srv_adm_st_chg_cause ca
>*/
>   if (s->check.state & CHK_ST_ENABLED) {
>   s->check.state &= ~CHK_ST_PAUSED;
> - s->check.health = s->check.rise; /* start OK but check 
> immediately */
> + if(s->init_state == SRV_INIT_STATE_DOWN) {
 ^^
missing space here. :-)

> + s->check.health = 0; /* checks must pass before 
> the server is considered UP */
> + } else {
> + s->check.health = s->check.rise; /* start OK 
> but check immediately */
> + }
>   }

Thanks!
Willy

Re: [PATCH] CLEANUP: assorted typo fixes in the code and comments

[Willy Tarreau]

On Mon, Aug 26, 2024 at 11:40:15PM +0200, Ilya Shipitsin wrote:
> This is 43rd iteration of typo fixes

Merged, thank you Ilya!
Willy

Re: Fwd: [PATCH v2 4/4] FEATURE: add MPTCP per address support

[Willy Tarreau]

On Fri, Aug 30, 2024 at 03:32:54PM +0200, Anthony Doeraene wrote:
> > Otherwise let me know if I can apply it as-is.
> 
> Seems good to me ! Thank you again for your reviews.

OK now pushed, thank you!

> > I'm very happy that after so long this topic has been alive, with your,
> > Matt's and Dorian's help we're finally reaching the end of the tunnel!
> 
> Thank you also for being open to these changes ! The fact that haproxy
> should support soon MPTCP could encourage even more application
> developers to use it, as it becomes easier for them to enable it on the
> server side :D. For the anecdote, we had a debate with some application
> developers that lacked support for MPTCP in haproxy just 3 days ago, that
> should help them too !

Ah! so if there are any bugs left, we should be aware soon ;-)

Have a nice week-end!
Willy

Re: [PATCH v2 4/4] FEATURE: add MPTCP per address support

[Willy Tarreau]

On Mon, Aug 26, 2024 at 11:50:27AM +0200, Aperence wrote:
> diff --git a/src/backend.c b/src/backend.c
> index 6956d9bfe..e4bd465e9 100644
> --- a/src/backend.c
> +++ b/src/backend.c
> @@ -1690,8 +1690,9 @@ int connect_server(struct stream *s)
>  
>   if (!srv_conn->xprt) {
>   /* set the correct protocol on the output stream connector */
> +
>   if (srv) {
> - if (conn_prepare(srv_conn, 
> protocol_lookup(srv_conn->dst->ss_family, PROTO_TYPE_STREAM, 0), srv->xprt)) {
> + if (conn_prepare(srv_conn, 
> protocol_lookup(srv_conn->dst->ss_family, PROTO_TYPE_STREAM, srv->alt_proto), 
> srv->xprt)) {
>   conn_free(srv_conn);
>   return SF_ERR_INTERNAL;
>   }

This one could have been left in a separate patch (like the 3/4) but as
we know that no other backend proto uses alt_proto right now, it really
has no impact.

Nothing else to say for the rest of the series. It's clean and looks
sane. I'm ready to merge it. I'd just adjust the severity subject tags
(MINOR for the first 2 and MEDIUM for the last two I think), and change
one word in the previous patch's commit message as mentioned.

If you have any last-minute change pending, feel free to suggest them
(I can apply them manually if they're trivial) or to resend the series.
Otherwise let me know if I can apply it as-is.

I'm very happy that after so long this topic has been alive, with your,
Matt's and Dorian's help we're finally reaching the end of the tunnel!

Thanks!
Willy

Re: Seeking Clarity on Timeout and Option Redispatch Settings

[Jesse Brink]

Lukas,

Thank you for your quick response. That explanation makes a lot of sense. I 
appreciate your time.

-Jesse

Get Outlook for iOS<https://aka.ms/o0ukef>

From: Lukas Tribus 
Sent: Friday, August 30, 2024 2:27:28 AM
To: J B ; haproxy 
Subject: Re: Seeking Clarity on Timeout and Option Redispatch Settings

Hello,

a connect timeout is used only *when it is actually needed*, that is
when we send SYN but get no response whatsoever from a backend.

However when the backend server clearly responds with a RST or ICMP
unreachable - which is the case when the is no application running on
the particular port, the error is passed immediately up the stack and
haproxy can handle it without waiting for a timeout.

If you want to test timeout settings, point somewhere where you don't
get a response.


Lukas

Re: [PATCH v2 3/4] REORG: use protocol when creating socket

[Willy Tarreau]

On Mon, Aug 26, 2024 at 11:50:26AM +0200, Aperence wrote:
> Use the protocol configured for a connection when creating the socket,
> instead of always using 0.
> 
> This change is needed to allow new protocol to be used when creating
> the sockets, such as MPTCP. Note however that this patch won't change
> anything for now, as the only value that proto->sock_prot could hold
> is IPPROTO_TCP, which has the same behavior as 0 when passed to socket.

To be precise, it's the only *other* value that can be IPPROTO_TCP,
because for unix sockets and socket pairs we're passing zero there,
and sock_prot is properly zero for them ;-)

I'll adjust that in the commit message myself if I take the series
as-is.

Willy

Re: [PATCH v2 4/4] FEATURE: add MPTCP per address support

[Willy Tarreau]

Hi Anthony,

On Mon, Aug 26, 2024 at 12:03:50PM +0200, Anthony Doeraene wrote:
> Hello,
> 
> Changelog compared to the previous patch:
> 
> - split some code to patches 1-3/4
> - use MPTCP with the backend if set in the backend config thanks to
> patches X/4
> - remove ".receivers" in the new MPTCP protocol structures.
> - split too large lines
> - fix family for 'mptcp6@' address

Thanks for the changes, that will help me, I'm back to reviewing the
changes again.

> Another question: Is it still needed to duplicate the mptcp protocol 
> structures
> from the tcp ones ? Could we not simply make a copy of the tcp ones and simply
> setting the name and sock_proto ?

It's better to avoid the copy. The reason is that it's too easy to
overlook something. For example there is a linked list element in the
struct which I used to mistakenly copy-paste, and that I recently
changed so that it's initialized by the register code. But if we later
add something similar, it will be very hard to spot the lack of special
case in the memcpy() code. The structs are not *that* large and remain
reasonably readable so let's have one per variant for now.

I also hesitated to set some defaults for many entries to avoid having
to declare all of them, but figured it would be hard not to set some
functions when we really want a NULL, and it would also make it harder
to figure all users of this or that function. I think what we have now
is far from being perfect but is a reasonably acceptable compromise in
fact.

Thanks!
Willy

Re: [PATCH] CLEANUP: haproxy: fix typos in code comment

[Willy Tarreau]

Hi Nicolas,

On Tue, Aug 27, 2024 at 10:18:51PM +0200, Nicolas CARPi wrote:
> Found these two typos while browsing the code :)
(...)
> Found this typo in macro name :)

Thank you, both patches applied.

> BTW, in mqtt.c mqtt_read_varint(), "off" is initialized to 0, but 
> initialized again in the for loop, which results in a dead assignment.
> 
> I believe this is done on purpose, as some sort of defensive
> programming, so I didn't touch it, but wanted to bring attention to it, 
> in case it is not something wanted.

Indeed it's not the prettiest but not a problem either. Sometimes
this can happen as the result of removing a check and an error path
for example. Let's leave it as is, it has no effect anyway :-)

Thank you!
Willy

Re: Seeking Clarity on Timeout and Option Redispatch Settings

[Lukas Tribus]

Hello,

a connect timeout is used only *when it is actually needed*, that is
when we send SYN but get no response whatsoever from a backend.

However when the backend server clearly responds with a RST or ICMP
unreachable - which is the case when the is no application running on
the particular port, the error is passed immediately up the stack and
haproxy can handle it without waiting for a timeout.

If you want to test timeout settings, point somewhere where you don't
get a response.


Lukas

Re: HAProxy returning 502 with SH--

[BJ Taylor]

Ok, will do. Thanks.

BJ


On Tue, Aug 27, 2024 at 10:42 AM Lukas Tribus  wrote:

> Also, before doing anything else, try using:
>
> tune.disable-zero-copy-forwarding or tune.h1.zero-copy-fwd-recv off
>
> as there is currently an open bug that doesn't fully match your case
> but is still close enough that it may be worth a try:
>
> https://github.com/haproxy/haproxy/issues/2665
>
>
>
> Lukas
>

Re: HAProxy returning 502 with SH--

[Lukas Tribus]

Also, before doing anything else, try using:

tune.disable-zero-copy-forwarding or tune.h1.zero-copy-fwd-recv off

as there is currently an open bug that doesn't fully match your case
but is still close enough that it may be worth a try:

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



Lukas

Re: HAProxy returning 502 with SH--

[Lukas Tribus]

Hello,

On Tue, 27 Aug 2024 at 18:09, BJ Taylor  wrote:
>
> Here are the 502 logs from the last run after the config changes.
>
> 2024-08-26T09:29:02.547581-06:00 testserver haproxy[284569]: <134>Aug 26 
> 09:29:02 haproxy[284569]: 192.168.69.101:45382 [26/Aug/2024:09:29:02.545] 
> www~ front3/pdafront32 0/0/0/-1/1 502 208 - - SH-- 5/5/3/3/0 0/0 
> {front3.domain.com|} "POST https://front3.domain.com/front1 HTTP/2.0"
> 2024-08-26T11:27:20.748921-06:00 testserver haproxy[284569]: <134>Aug 26 
> 11:27:20 haproxy[284569]: 192.168.69.101:50606 [26/Aug/2024:11:27:20.746] 
> www~ front3/pdafront32 0/0/0/-1/1 502 208 - - SH-- 5/5/3/3/0 0/0 
> {front3.domain.com|} "POST https://front3.domain.com/front1 HTTP/2.0"
> 2024-08-26T14:11:11.289987-06:00 testserver haproxy[284569]: <134>Aug 26 
> 14:11:11 haproxy[284569]: 192.168.69.101:40516 [26/Aug/2024:14:11:11.285] 
> www~ front3/pdafront32 0/0/0/-1/2 502 208 - - SH-- 15/15/7/7/0 0/0 
> {front3.domain.com|} "POST https://front3.domain.com/front1 HTTP/2.0"
> 2024-08-26T17:40:55.801154-06:00 testserver haproxy[284569]: <134>Aug 26 
> 17:40:55 haproxy[284569]: 192.168.69.101:53952 [26/Aug/2024:17:40:55.798] 
> www~ front3/pdafront32 0/0/0/-1/1 502 208 - - SH-- 10/10/1/1/0 0/0 
> {front3.domain.com|} "POST https://front3.domain.com/front1 HTTP/2.0"

This indicates that your backend applications crashes or at least does
not complete the HTTP response header after 208 bytes.

It possible that "show errors" on the haproxy admin socket gives your
more insight into what the HTTP response of your server looks like
(and where it suddenly aborts exactly after those 208 bytes).

Try switching off H2 in the backend, to see if this is H2 related.

If you can switch off SSL on the backend and you can still reproduce
the issue, you may have an easier time debugging this with network
traces.

Otherwise if you have no possibilities to troubleshoot at the backend
application, show errors is not useful and you cannot disable SSL on
the backend, you need to be able to decrypt the backend traffic from a
network trace. Reproducing with a non-FS cipher will allow you to
decrypt the SSL traffic with the certificates private key; otherwise
you have to use client random logging [1] and then decrypt the traffic
in wireshark before analyzing what happens in those last bytes of the
208 byte incomplete HTTP response.



Regards,
Lukas


[1] http://docs.haproxy.org/3.0/configuration.html#3.2-tune.ssl.keylog

Re: HAProxy returning 502 with SH--

[BJ Taylor]

Here are the 502 logs from the last run after the config changes.

2024-08-26T09:29:02.547581-06:00 testserver haproxy[284569]: <134>Aug 26
09:29:02 haproxy[284569]: 192.168.69.101:45382 [26/Aug/2024:09:29:02.545]
www~ front3/pdafront32 0/0/0/-1/1 502 208 - - SH-- 5/5/3/3/0 0/0 {
front3.domain.com|} "POST https://front3.domain.com/front1 HTTP/2.0"
2024-08-26T11:27:20.748921-06:00 testserver haproxy[284569]: <134>Aug 26
11:27:20 haproxy[284569]: 192.168.69.101:50606 [26/Aug/2024:11:27:20.746]
www~ front3/pdafront32 0/0/0/-1/1 502 208 - - SH-- 5/5/3/3/0 0/0 {
front3.domain.com|} "POST https://front3.domain.com/front1 HTTP/2.0"
2024-08-26T14:11:11.289987-06:00 testserver haproxy[284569]: <134>Aug 26
14:11:11 haproxy[284569]: 192.168.69.101:40516 [26/Aug/2024:14:11:11.285]
www~ front3/pdafront32 0/0/0/-1/2 502 208 - - SH-- 15/15/7/7/0 0/0 {
front3.domain.com|} "POST https://front3.domain.com/front1 HTTP/2.0"
2024-08-26T17:40:55.801154-06:00 testserver haproxy[284569]: <134>Aug 26
17:40:55 haproxy[284569]: 192.168.69.101:53952 [26/Aug/2024:17:40:55.798]
www~ front3/pdafront32 0/0/0/-1/1 502 208 - - SH-- 10/10/1/1/0 0/0 {
front3.domain.com|} "POST https://front3.domain.com/front1 HTTP/2.0"

I'm open to suggestions.
Thanks,
BJ

On Fri, Aug 23, 2024 at 2:20 PM BJ Taylor  wrote:

> I will make the suggested changes and try again. Here are the failure logs
> for the last run. Queues are 0, current connections are not all that high.
>
> Aug 22 01:12:12 haproxy[87118]:
>> {"host":"testserver","ident":"haproxy","pid":87118,"timestamp":"22/Aug/2024:01:12:12
>> -0600","haproxy":{"connections":{"active":27,"frontend":27,"backend":13,"server":13},"queue":{"backend":0,"server":0},"timing_ms":{"time_to_end_of_headers":0,"client_request_send_time":226,"queue_wait_time":0,"server_wait_time":0,"server_response_send_time":-1,"response_time":-1,"session_duration":226},"termination_state":"SH--","retries":0,"network":{"client_ip":"192.168.69.101","client_port":47794,"frontend_ip":"192.168.39.5","frontend_port":443},"ssl":{"version":"TLSv1.3","ciphers":"TLS_AES_128_GCM_SHA256"},"request":{"method":"POST","uri":"\/front1","protocol":"HTTP/2.0","header":{"host":"
>> front3.domain.com",
>>  
>> "xforwardfor":"-","referer":"-"}},"name":{"frontend":"www~","backend":"front3",
>>"server":"testbackend1"},
>>  
>> "response":{"status_code":502,"header":{"xrequestid":"-"}},"bytes":{"uploaded":65785,"read":208}}}
>> Aug 22 02:57:48 haproxy[87118]:
>> {"host":"testserver","ident":"haproxy","pid":87118,"timestamp":"22/Aug/2024:02:57:48
>> -0600","haproxy":{"connections":{"active":18,"frontend":18,"backend":3,
>> "server":3
>> },"queue":{"backend":0,"server":0},"timing_ms":{"time_to_end_of_headers":0,"client_request_send_time":226,"queue_wait_time":0,"server_wait_time":0,"server_response_send_time":-1,"response_time":-1,"session_duration":226},"termination_state":"SH--","retries":0,"network":{"client_ip":"192.168.69.101","client_port":51728,"frontend_ip":"192.168.39.5","frontend_port":443},"ssl":{"version":"TLSv1.3","ciphers":"TLS_AES_128_GCM_SHA256"},"request":{"method":"POST","uri":"\/front1","protocol":"HTTP/2.0","header":{"host":"
>> front3.domain.com",
>>  
>> "xforwardfor":"-","referer":"-"}},"name":{"frontend":"www~","backend":"front3",
>>"server":"testbackend1"},
>>  
>> "response":{"status_code":502,"header":{"xrequestid":"-"}},"bytes":{"uploaded":250,
>>  "read":208}}}
>> Aug 22 03:22:34 haproxy[87118]:
>> {"host":"testserver","ident":"haproxy","pid":87118,"timestamp":"22/Aug/2024:03:22:34
>> -0600","haproxy":{"connections":{"active":14,"frontend":14,"backend":4,
>> "server":4
>> },"queue":{"backend":0,"server":0},"timing_ms":{"time_to_end_of_headers":0,"client_request_send_time":222,"queue_wait_time":0,"server_wait_time":0,"server_response_send_time":-1,"response_time":-1,"session_duration":223},"termination_state":"SH--","retries":0,"network":{"client_ip":"192.168.69.101","client_port":36912,"frontend_ip":"192.168.39.5","frontend_port":443},"ssl":{"version":"TLSv1.3","ciphers":"TLS_AES_128_GCM_SHA256"},"request":{"method":"POST","uri":"\/front1","protocol":"HTTP/2.0","header":{"host":"
>> front3.domain.com",
>>  
>> "xforwardfor":"-","referer":"-"}},"name":{"frontend":"www~","backend":"front3",
>>"server":"testbackend1"},
>>  
>> "response":{"status_code":502,"header":{"xrequestid":"-"}},"bytes":{"uploaded":65785,"read":208}}}
>> Aug 21 16:26:12 haproxy[87118]:
>> {"host":"testserver","ident":"haproxy","pid":87118,"timestamp":"21/Aug/2024:16:26:12
>> -0600","haproxy":{"connections":{"active":8, "frontend":8, "backend":3,
>> "server":3
>> },"queue":{"backend":0,"server":0},"timing_ms":{"time_to_end_of_headers":0,"client_request_send_time":224,"queue_wait_time":0,"server_wait_time":0,"server_response_send_time":-1,"response_time":-1,"session_duration":224},"termination_state":"SH--","retries":0,"network":{"client_ip":"192.168.69.101","client_port":35664,"frontend_ip":"192.168.39.5","frontend_port":443},"ssl":{"version":"TLS

Re: [PATCH v2 4/4] FEATURE: add MPTCP per address support

[Anthony Doeraene]

Hello,

Changelog compared to the previous patch:

- split some code to patches 1-3/4
- use MPTCP with the backend if set in the backend config thanks to
patches X/4
- remove ".receivers" in the new MPTCP protocol structures.
- split too large lines
- fix family for 'mptcp6@' address

Another question: Is it still needed to duplicate the mptcp protocol structures
from the tcp ones ? Could we not simply make a copy of the tcp ones and simply
setting the name and sock_proto ?

Doeraene Anthony

On Mon, Aug 26, 2024 at 11:51 AM Aperence
 wrote:
>
> Multipath TCP (MPTCP), standardized in RFC8684 [1], is a TCP extension
> that enables a TCP connection to use different paths.
>
> Multipath TCP has been used for several use cases. On smartphones, MPTCP
> enables seamless handovers between cellular and Wi-Fi networks while
> preserving established connections. This use-case is what pushed Apple
> to use MPTCP since 2013 in multiple applications [2]. On dual-stack
> hosts, Multipath TCP enables the TCP connection to automatically use the
> best performing path, either IPv4 or IPv6. If one path fails, MPTCP
> automatically uses the other path.
>
> To benefit from MPTCP, both the client and the server have to support
> it. Multipath TCP is a backward-compatible TCP extension that is enabled
> by default on recent Linux distributions (Debian, Ubuntu, Redhat, ...).
> Multipath TCP is included in the Linux kernel since version 5.6 [3]. To
> use it on Linux, an application must explicitly enable it when creating
> the socket. No need to change anything else in the application.
>
> This attached patch adds MPTCP per address support, to be used with:
>
>   mptcp{,4,6}@[:port1[-port2]]
>
> MPTCP v4 and v6 protocols have been added: they are mainly a copy of the
> TCP ones, with small differences: names, proto, and receivers lists.
>
> These protocols are stored in __protocol_by_family, as an alternative to
> TCP, similar to what has been done with QUIC. By doing that, the size of
> __protocol_by_family has not been increased, and it behaves like TCP.
>
> MPTCP is both supported for the frontend and backend sides.
>
> Also added an example of configuration using mptcp along with a backend
> allowing to experiment with it.
>
> Note that this is a re-implementation of Björn's work from 3 years ago
> [4], when haproxy's internals were probably less ready to deal with
> this, causing his work to be left pending for a while.
>
> Currently, the TCP_MAXSEG socket option doesn't seem to be supported
> with MPTCP [5]. This results in a warning when trying to set the MSS of
> sockets in proto_tcp:tcp_bind_listener.
>
> This can be resolved by adding two new variables:
> sock_inet(6)_mptcp_maxseg_default that will hold the default
> value of the TCP_MAXSEG option. Note that for the moment, this
> will always be -1 as the option isn't supported. However, in the
> future, when the support for this option will be added, it should
> contain the correct value for the MSS, allowing to correctly
> set the TCP_MAXSEG option.
>
> Link: https://www.rfc-editor.org/rfc/rfc8684.html [1]
> Link: https://www.tessares.net/apples-mptcp-story-so-far/ [2]
> Link: https://www.mptcp.dev [3]
> Link: https://github.com/haproxy/haproxy/issues/1028 [4]
> Link: https://github.com/multipath-tcp/mptcp_net-next/issues/515 [5]
>
> Co-authored-by: Dorian Craps 
> Co-authored-by: Matthieu Baerts (NGI0) 
> ---
>  doc/configuration.txt   |  21 +++
>  examples/mptcp-backend.py   |  22 
>  examples/mptcp.cfg  |  23 
>  include/haproxy/compat.h|  10 
>  include/haproxy/sock_inet.h |   8 +++
>  src/backend.c   |   3 +-
>  src/proto_tcp.c | 108 ++--
>  src/protocol.c  |   3 +-
>  src/sock.c  |   5 +-
>  src/sock_inet.c |  30 ++
>  src/tools.c |  21 +++
>  11 files changed, 246 insertions(+), 8 deletions(-)
>  create mode 100644 examples/mptcp-backend.py
>  create mode 100644 examples/mptcp.cfg
>
> diff --git a/doc/configuration.txt b/doc/configuration.txt
> index 213febb76..802d5fe3f 100644
> --- a/doc/configuration.txt
> +++ b/doc/configuration.txt
> @@ -28255,6 +28255,27 @@ report this to the maintainers.
>   range can or must be specified.
>   It is considered as an alias of 
> 'stream+ipv4@'.
>
> +'mptcp@[:port1[-port2]]' following  is considered as an 
> IPv4
> +  or IPv6 address depending of the syntax but
> +  socket type and transport method is forced 

Re: [PATCH 1/2] FEATURE: add MPTCP per address support

[Anthony Doeraene]

Hello,

On Fri, Aug 23, 2024 at 4:36 PM Willy Tarreau  wrote:
> > diff --git a/include/haproxy/compat.h b/include/haproxy/compat.h
> > index 3829060b7..2347d62cb 100644
> > --- a/include/haproxy/compat.h
> > +++ b/include/haproxy/compat.h
> > @@ -317,6 +317,11 @@ typedef struct { } empty_t;
> >  #define queue _queue
> >  #endif
> >
> > +/* only Linux defines IPPROTO_MPTCP */
> > +#ifndef IPPROTO_MPTCP
> > +#define IPPROTO_MPTCP 262
> > +#endif
>
> Stupid open question: do we really want to define it for other
> OSes ? I really don't care, to be honest.

It simplifies a lot the code, by not having to put nasty #ifdef IPPROTO_MPTCP
everywhere, that would make quickly the code unreadable. However, we made
sure that it isn't possible to create sockets with IPPROTO_MPTCP, to avoid
users having some difficult to understand error because the protocol isn't
supported on their platform

Thank you for your quick feedback ! A second version of these patches should
soon follow, I'm only adjusting the last details.

Have a nice day,
Doeraene Anthony

On Fri, Aug 23, 2024 at 4:36 PM Willy Tarreau  wrote:
>
> On Fri, Aug 23, 2024 at 03:34:09PM +0200, Aperence wrote:
> (...)
> > MPTCP is both supported for the frontend and backend sides.
>
> Great!
>
> > Also added an example of configuration using mptcp along with a backend
> > allowing to experiment with it.
>
> Thanks for thinking about testing ;-)
>
> > Note that this is a re-implementation of Björn's work from 3 years ago
> > [4], when haproxy's internals were probably less ready to deal with
> > this, causing his work to be left pending for a while.
>
> Thanks for mentioning his initial work!
>
> > diff --git a/include/haproxy/compat.h b/include/haproxy/compat.h
> > index 3829060b7..2347d62cb 100644
> > --- a/include/haproxy/compat.h
> > +++ b/include/haproxy/compat.h
> > @@ -317,6 +317,11 @@ typedef struct { } empty_t;
> >  #define queue _queue
> >  #endif
> >
> > +/* only Linux defines IPPROTO_MPTCP */
> > +#ifndef IPPROTO_MPTCP
> > +#define IPPROTO_MPTCP 262
> > +#endif
>
> Stupid open question: do we really want to define it for other
> OSes ? I really don't care, to be honest.
>
> > diff --git a/src/backend.c b/src/backend.c
> > index 6956d9bfe..6b865768e 100644
> > --- a/src/backend.c
> > +++ b/src/backend.c
> > @@ -1690,8 +1690,15 @@ int connect_server(struct stream *s)
> >
> >   if (!srv_conn->xprt) {
> >   /* set the correct protocol on the output stream connector */
> > + int mptcp = 0;
> > +
> > + /* cli_conn can be NULL when the origin of the stream isn't a
> > +  * connection, there's no reason to use MPTCP in this case */
> > + if (cli_conn && cli_conn->ctrl)
> > + mptcp = cli_conn->ctrl->sock_prot == IPPROTO_MPTCP;
>
> I don't understand this part, it seems to imply a relation between the
> client and the server. I'm not seeing any case where this could be valid
> since the config of the client and the server are two totally independent
> things.
>
> I'm suspecting that the difficulty here is to know that the server's
> address was configured as MPTCP and that since you couldn't find where
> to store that piece of info, instead it's inherited from the client. But
> that cannot be right as it's really a proxy and there's no relation
> between the protocols used on each side. For example it's valid and
> normal to have QUIC on the front and HTTP/2 over TCP (or MPTCP soon)
> on the back.
>
> If that's the issue, I suspect that we'll have to store the info in
> the struct server itself that we want to use mptcp. BTW, based on
> the extension you brought to protocol_lookup(), maybe we can have a
> more generic field like "alt_proto" or something like this that is
> passed directly to protocol_lookup(). This info needs to be
> retrieved from the address parser called by _srv_parse_init(). I'm
> seeing nowhere to store the "alt" value, and that might be an
> indication that str2sa_range() needs to be extended to take yet
> another arg to return that one, which you're already setting to
> !!mptcp during the lookup.
>
> As such I would suggest the following:
>   - a preliminary patch extending str2sa_range() to add an int *alt
> before the last "opts" parameter, and that is filled with the
> value passed to protocol_lookup() during the function if the
> pointer is not NULL. You can put in that patch the c

Re: [PATCH 3/3] CI: QUIC Interop AWS-LC: enable ngtcp2 client

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

yep, I forgot that json requires a comma   (that workflow is scheduled, I
overlooked on commit).

I'll send fix soon (also, we agreed with haproxyFred upon several things, I
split changes into several commits in purpose)

сб, 24 авг. 2024 г. в 19:22, Willy Tarreau :

> On Sat, Aug 24, 2024 at 07:13:09PM +0200, Willy Tarreau wrote:
> > On Sat, Aug 24, 2024 at 07:02:30PM +0200,  ??? wrote:
> > > the reason of adding ngtcp2 is
> > >
> > > "let's add and see how it goes".
> > >
> > > and that is what we agreed in GH issue :)
> >
> > That's fine by me, thank you!
>
> Apparently it doesn't like it much:
>
>   https://github.com/haproxy/haproxy/actions/runs/10540196449/workflow
>
> Willy
>

Re: [PATCH 3/3] CI: QUIC Interop AWS-LC: enable ngtcp2 client

[Willy Tarreau]

On Sat, Aug 24, 2024 at 07:13:09PM +0200, Willy Tarreau wrote:
> On Sat, Aug 24, 2024 at 07:02:30PM +0200,  ??? wrote:
> > the reason of adding ngtcp2 is
> > 
> > "let's add and see how it goes".
> > 
> > and that is what we agreed in GH issue :)
> 
> That's fine by me, thank you!

Apparently it doesn't like it much:

  https://github.com/haproxy/haproxy/actions/runs/10540196449/workflow

Willy

Re: [PATCH 3/3] CI: QUIC Interop AWS-LC: enable ngtcp2 client

[Willy Tarreau]

On Sat, Aug 24, 2024 at 07:02:30PM +0200,  ??? wrote:
> the reason of adding ngtcp2 is
> 
> "let's add and see how it goes".
> 
> and that is what we agreed in GH issue :)

That's fine by me, thank you!
Willy

Re: [PATCH 3/3] CI: QUIC Interop AWS-LC: enable ngtcp2 client

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

the reason of adding ngtcp2 is

"let's add and see how it goes".

and that is what we agreed in GH issue :)

сб, 24 авг. 2024 г. в 18:45, Willy Tarreau :

> On Sat, Aug 24, 2024 at 06:15:59PM +0200,  ??? wrote:
> > it is pretty much about it.
> > short commit message "CI: QUIC Interop AWS-LC: enable ngtcp2 client"
> > describes what is done, GH issue is optional.
>
> But it does not explain why (or what the purpose is). As I often say
> it, commit messages must explain the intent, much more than the what.
> I know it's less important for CI which is a bit of a side project,
> but regardless, I'm wondering things like "what's the impact, will
> this regularly break some tests or take more time to build?" then
> "but if so, what's the purpose first?".
>
> I'm perfectly fine with simple explanations like "we have no other
> QUIC client and we need one", or "it's the simplest QUIC client we've
> found for now", or "ngtcp2's client provides stats others do not", or
> "let's start with this one for now, we may change it later" etc.
> See ?
>
> Thanks!
> Willy
>

Re: [PATCH 3/3] CI: QUIC Interop AWS-LC: enable ngtcp2 client

[Willy Tarreau]

On Sat, Aug 24, 2024 at 06:15:59PM +0200,  ??? wrote:
> it is pretty much about it.
> short commit message "CI: QUIC Interop AWS-LC: enable ngtcp2 client"
> describes what is done, GH issue is optional.

But it does not explain why (or what the purpose is). As I often say
it, commit messages must explain the intent, much more than the what.
I know it's less important for CI which is a bit of a side project,
but regardless, I'm wondering things like "what's the impact, will
this regularly break some tests or take more time to build?" then
"but if so, what's the purpose first?".

I'm perfectly fine with simple explanations like "we have no other
QUIC client and we need one", or "it's the simplest QUIC client we've
found for now", or "ngtcp2's client provides stats others do not", or
"let's start with this one for now, we may change it later" etc.
See ?

Thanks!
Willy

Re: [PATCH 3/3] CI: QUIC Interop AWS-LC: enable ngtcp2 client

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

it is pretty much about it.
short commit message "CI: QUIC Interop AWS-LC: enable ngtcp2 client"
describes what is done, GH issue is optional.

сб, 24 авг. 2024 г. в 17:17, Willy Tarreau :

> Hi Ilya,
>
> On Sat, Aug 24, 2024 at 03:55:45PM +0200, Ilya Shipitsin wrote:
> > GH issue: https://github.com/haproxy/haproxy/issues/2688
>
> While it's fine to put references to GH issues to ease tracking, please
> always leave a bit of information in the commit message about what the
> intent of the patch is. Here I really have no idea, the issue above is
> long and discusses various things. In addition, if one day GH issues
> become inaccessible for any reason, the history is lost.
>
> Thanks!
> Willy
>

Re: [PATCH 3/3] CI: QUIC Interop AWS-LC: enable ngtcp2 client

[Willy Tarreau]

Hi Ilya,

On Sat, Aug 24, 2024 at 03:55:45PM +0200, Ilya Shipitsin wrote:
> GH issue: https://github.com/haproxy/haproxy/issues/2688

While it's fine to put references to GH issues to ease tracking, please
always leave a bit of information in the commit message about what the
intent of the patch is. Here I really have no idea, the issue above is
long and discusses various things. In addition, if one day GH issues
become inaccessible for any reason, the history is lost.

Thanks!
Willy

Re: HAProxy returning 502 with SH--

[BJ Taylor]

I will make the suggested changes and try again. Here are the failure logs
for the last run. Queues are 0, current connections are not all that high.

Aug 22 01:12:12 haproxy[87118]:
> {"host":"testserver","ident":"haproxy","pid":87118,"timestamp":"22/Aug/2024:01:12:12
> -0600","haproxy":{"connections":{"active":27,"frontend":27,"backend":13,"server":13},"queue":{"backend":0,"server":0},"timing_ms":{"time_to_end_of_headers":0,"client_request_send_time":226,"queue_wait_time":0,"server_wait_time":0,"server_response_send_time":-1,"response_time":-1,"session_duration":226},"termination_state":"SH--","retries":0,"network":{"client_ip":"192.168.69.101","client_port":47794,"frontend_ip":"192.168.39.5","frontend_port":443},"ssl":{"version":"TLSv1.3","ciphers":"TLS_AES_128_GCM_SHA256"},"request":{"method":"POST","uri":"\/front1","protocol":"HTTP/2.0","header":{"host":"
> front3.domain.com",
>  
> "xforwardfor":"-","referer":"-"}},"name":{"frontend":"www~","backend":"front3",
>"server":"testbackend1"},
>  
> "response":{"status_code":502,"header":{"xrequestid":"-"}},"bytes":{"uploaded":65785,"read":208}}}
> Aug 22 02:57:48 haproxy[87118]:
> {"host":"testserver","ident":"haproxy","pid":87118,"timestamp":"22/Aug/2024:02:57:48
> -0600","haproxy":{"connections":{"active":18,"frontend":18,"backend":3,
> "server":3
> },"queue":{"backend":0,"server":0},"timing_ms":{"time_to_end_of_headers":0,"client_request_send_time":226,"queue_wait_time":0,"server_wait_time":0,"server_response_send_time":-1,"response_time":-1,"session_duration":226},"termination_state":"SH--","retries":0,"network":{"client_ip":"192.168.69.101","client_port":51728,"frontend_ip":"192.168.39.5","frontend_port":443},"ssl":{"version":"TLSv1.3","ciphers":"TLS_AES_128_GCM_SHA256"},"request":{"method":"POST","uri":"\/front1","protocol":"HTTP/2.0","header":{"host":"
> front3.domain.com",
>  
> "xforwardfor":"-","referer":"-"}},"name":{"frontend":"www~","backend":"front3",
>"server":"testbackend1"},
>  
> "response":{"status_code":502,"header":{"xrequestid":"-"}},"bytes":{"uploaded":250,
>  "read":208}}}
> Aug 22 03:22:34 haproxy[87118]:
> {"host":"testserver","ident":"haproxy","pid":87118,"timestamp":"22/Aug/2024:03:22:34
> -0600","haproxy":{"connections":{"active":14,"frontend":14,"backend":4,
> "server":4
> },"queue":{"backend":0,"server":0},"timing_ms":{"time_to_end_of_headers":0,"client_request_send_time":222,"queue_wait_time":0,"server_wait_time":0,"server_response_send_time":-1,"response_time":-1,"session_duration":223},"termination_state":"SH--","retries":0,"network":{"client_ip":"192.168.69.101","client_port":36912,"frontend_ip":"192.168.39.5","frontend_port":443},"ssl":{"version":"TLSv1.3","ciphers":"TLS_AES_128_GCM_SHA256"},"request":{"method":"POST","uri":"\/front1","protocol":"HTTP/2.0","header":{"host":"
> front3.domain.com",
>  
> "xforwardfor":"-","referer":"-"}},"name":{"frontend":"www~","backend":"front3",
>"server":"testbackend1"},
>  
> "response":{"status_code":502,"header":{"xrequestid":"-"}},"bytes":{"uploaded":65785,"read":208}}}
> Aug 21 16:26:12 haproxy[87118]:
> {"host":"testserver","ident":"haproxy","pid":87118,"timestamp":"21/Aug/2024:16:26:12
> -0600","haproxy":{"connections":{"active":8, "frontend":8, "backend":3,
> "server":3
> },"queue":{"backend":0,"server":0},"timing_ms":{"time_to_end_of_headers":0,"client_request_send_time":224,"queue_wait_time":0,"server_wait_time":0,"server_response_send_time":-1,"response_time":-1,"session_duration":224},"termination_state":"SH--","retries":0,"network":{"client_ip":"192.168.69.101","client_port":35664,"frontend_ip":"192.168.39.5","frontend_port":443},"ssl":{"version":"TLSv1.3","ciphers":"TLS_AES_128_GCM_SHA256"},"request":{"method":"POST","uri":"\/front1","protocol":"HTTP/2.0","header":{"host":"
> front3.domain.com",
>  
> "xforwardfor":"-","referer":"-"}},"name":{"frontend":"www~","backend":"front3",
>"server":"testbackend1"},
>  
> "response":{"status_code":502,"header":{"xrequestid":"-"}},"bytes":{"uploaded":65785,"read":208}}}
> Aug 21 20:43:51 haproxy[87118]:
> {"host":"testserver","ident":"haproxy","pid":87118,"timestamp":"21/Aug/2024:20:43:51
> -0600","haproxy":{"connections":{"active":28,"frontend":28,"backend":4,
> "server":4
> },"queue":{"backend":0,"server":0},"timing_ms":{"time_to_end_of_headers":0,"client_request_send_time":101,"queue_wait_time":0,"server_wait_time":0,"server_response_send_time":-1,"response_time":-1,"session_duration":104},"termination_state":"SH--","retries":0,"network":{"client_ip":"192.168.6.3",
>
> "client_port":39160,"frontend_ip":"192.168.39.5","frontend_port":443},"ssl":{"version":"TLSv1.3","ciphers":"TLS_AES_128_GCM_SHA256"},"request":{"method":"POST","uri":"\/",
>  "protocol":"HTTP/2.0","header":{"host":"front1.domain.com",
>  
> "xforwardfor":"-","referer":"-"}},"name":{"frontend":"www~","backend":"front1",
>"server":"testbackend2"},
>  
> "response":{"status_code":502,"header":{"xrequestid":"-"}},"bytes":{"uploaded":65848,"read

Re: HAProxy returning 502 with SH--

[Lukas Tribus]

On Fri, 23 Aug 2024 at 18:55, BJ Taylor  wrote:
>
> We are trying to deploy HAProxy into our environment. We have a script that
> does some 600k api calls during approximately 24 hours.

How many concurrent connections / transactions though?


>  During that time, when haproxy is in place, there are a handful (8-12) of
> responses that come back as 502 with SH--.

As per the documentation:

 SH   The server aborted before sending its full HTTP response headers, or
  it crashed while processing the request. Since a server aborting at
  this moment is very rare, it would be wise to inspect its logs to
  control whether it crashed and why. The logged request may indicate a
  small set of faulty requests, demonstrating bugs in the application.
  Sometimes this might also be caused by an IDS killing the connection
  between HAProxy and the server.


You should disable custom format logging, enable httplog format and
share the affected SH log line.



>tune.bufsize 8388608
>tune.maxrewrite 1024

bufsize should usually be 16K maybe 32K for some specific application
requiring huge headers, but not 8M. I think 8M is unreasonable and I
think it will lead to issues one way or another.


>timeout connect 86400s
>timeout client  86400s
>timeout server  86400s

1 day long timeouts will probably also lead to an issue at some point,
due to sessions not expiring.




Lukas

Re: [PATCH 0/2] Add MPTCP to haproxy

[Matthieu Baerts]

On 23/08/2024 17:20, Willy Tarreau wrote:
> On Fri, Aug 23, 2024 at 05:11:11PM +0200, Matthieu Baerts wrote:

(...)

>> Maybe a new socket option would be better if the idea is only to
>> silently drop connections? :)
> 
> Yes, probably. Right now it's done directly in the action itself
> (tcp_exec_action_silent_drop()), and we already detect if the option
> is supported, so that would just be a matter of checking more
> explicitly for conn->ctrl->proto == mptcp and using the other option.

If such option is added, I think it is probably better to add it at TCP
level (or socket level), and then adds its support for MPTCP socket. But
yes, good to know it is easy to add alternatives there!

> In any case the code *tries* to make use of this, and will fall back
> to other methods. One of them is to try to change the TTL of the
> socket before closing (so that the RST doesn't reach the client).
> And when all of this fails, the connection is closed anyway, so it's
> just not silent. As such, there's no harm at all in not supporting this
> at all, it's just less optimal.

I sent patches more than one year ago to support these socket options to
modify the TTL in v4 and v6, as part of a refactoring, and they are
still not applied :(

(/me updates his TODO list)

>> (If these botnets are using "plain" TCP, the TCP_REPAIR socket option
>> will work!)
> 
> Ah, good to know! I guess we still have quite some time ahead before
> they start playing with mptcp by default, so for most use cases it will
> be fine!

I guess yes :)

(and currently MPTCP is not supported on Windows)

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

Re: [PATCH 0/2] Add MPTCP to haproxy

[Willy Tarreau]

On Fri, Aug 23, 2024 at 05:11:11PM +0200, Matthieu Baerts wrote:
> >>> With that said, from an implementation perspective, it would seem right
> >>> to make sure that most TCP tunables also work with MPTCP.
> >>
> >> That's what we tried to do. All "common" ones are supported, but it is
> >> not always easy to define what is "common" and what is not! There are
> >> many obscure/specific socket options out there :)
> > 
> > I agree :-) For example we're using other horrors such as TCP_REPAIR,
> > which was initially defined for CRIU, and that we're using to silently
> > destroy a connection without responding. It's extremely effective against
> > HTTP botnets as they keep their connection pending and do not consume
> > anything locally. I don't know if you have it, but if you're interested
> > in giving it a try, I can help you set it up in haproxy for a test.
> 
> Wow, nice bazooka :-)

:-)

> Here is what we can currently find in the MPTCP code in the kernel:
> 
>   /* TCP_REPAIR, TCP_REPAIR_QUEUE, TCP_QUEUE_SEQ, TCP_REPAIR_OPTIONS,
>* TCP_REPAIR_WINDOW are not supported, better avoid this mess
>*/
> 
> Maybe a new socket option would be better if the idea is only to
> silently drop connections? :)

Yes, probably. Right now it's done directly in the action itself
(tcp_exec_action_silent_drop()), and we already detect if the option
is supported, so that would just be a matter of checking more
explicitly for conn->ctrl->proto == mptcp and using the other option.

In any case the code *tries* to make use of this, and will fall back
to other methods. One of them is to try to change the TTL of the
socket before closing (so that the RST doesn't reach the client).
And when all of this fails, the connection is closed anyway, so it's
just not silent. As such, there's no harm at all in not supporting this
at all, it's just less optimal.

> (If these botnets are using "plain" TCP, the TCP_REPAIR socket option
> will work!)

Ah, good to know! I guess we still have quite some time ahead before
they start playing with mptcp by default, so for most use cases it will
be fine!

Willy

Re: [PATCH 0/2] Add MPTCP to haproxy

[Matthieu Baerts]

On 23/08/2024 16:42, Willy Tarreau wrote:
> On Fri, Aug 23, 2024 at 04:13:16PM +0200, Matthieu Baerts wrote:

(...)

>>> I'll comment on each patch separately,
>>
>> Thank you, please take your time!
> 
> That's what I'm doing but I really want to make sure we won't discover
> last-minute show-stoppers that require important changes. It's still
> possible to do some important ones for a few weeks, so let's tackle all
> roadblocks while we can. I really feel like we're on a good track now!

It makes sense, thank you for having done the reviews so quickly!

I will check with Anthony if he needs some help there.

(...)

>>> With that said, from an implementation perspective, it would seem right
>>> to make sure that most TCP tunables also work with MPTCP.
>>
>> That's what we tried to do. All "common" ones are supported, but it is
>> not always easy to define what is "common" and what is not! There are
>> many obscure/specific socket options out there :)
> 
> I agree :-) For example we're using other horrors such as TCP_REPAIR,
> which was initially defined for CRIU, and that we're using to silently
> destroy a connection without responding. It's extremely effective against
> HTTP botnets as they keep their connection pending and do not consume
> anything locally. I don't know if you have it, but if you're interested
> in giving it a try, I can help you set it up in haproxy for a test.

Wow, nice bazooka :-)

Here is what we can currently find in the MPTCP code in the kernel:

  /* TCP_REPAIR, TCP_REPAIR_QUEUE, TCP_QUEUE_SEQ, TCP_REPAIR_OPTIONS,
   * TCP_REPAIR_WINDOW are not supported, better avoid this mess
   */

Maybe a new socket option would be better if the idea is only to
silently drop connections? :)

(If these botnets are using "plain" TCP, the TCP_REPAIR socket option
will work!)

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

Re: [PATCH 2/2] BUG/MINOR: fix warning when setting MSS with MPTCP

[Willy Tarreau]

On Fri, Aug 23, 2024 at 04:50:33PM +0200, Willy Tarreau wrote:
> > @@ -494,6 +498,30 @@ static void sock_inet_prepare()
> >  #endif
> > close(fd);
> > }
> > +
> > +#ifdef __linux__
> 
> Here I think a short comment is deserved to explain why __linux__, because
> it's the same choice that was made in proto_tcp.c.
> 
> Or better, you could probably do that in compat.h:
> 
> #ifdef __linux__
> # define HA_HAVE_MPTCP 1
> # ifndef IPPROTO_TCP
> #  define IPPROTO_TCP ...
> # endif
> #endif
> 
> and use #ifdef HA_HAVE_MPTCP here and around the variables above. That
> would make it quite clear about what we're really checking for here.

Oh and BTW adding a link in a comment to the issue that Matthieu just
opened regarding TCP_MSS support would help the future us rediscovering
that code decide whether to remerge that with TCP or not once fully
supported everywhere.

Willy

Re: [PATCH 2/2] BUG/MINOR: fix warning when setting MSS with MPTCP

[Willy Tarreau]

On Fri, Aug 23, 2024 at 03:34:10PM +0200, Aperence wrote:
> Currently, the TCP_MAXSEG socket option doesn't seem to be supported
> with MPTCP. This results in a warning when trying to set the MSS of
> sockets in proto_tcp:tcp_bind_listener.
> 
> This can be resolved by adding two new variables:
> sock_inet(6)_mptcp_maxseg_default that will hold the default
> value of the TCP_MAXSEG option. Note that for the moment, this
> will always be -1 as the option isn't supported. However, in the
> future, when the support for this option will be added, it should
> contain the correct value for the MSS, allowing to correctly
> set the TCP_MAXSEG option.

I'm perfectly fine with the whole patch except that it's not logical
to add it as a bug fix after the first one which raises the API
incompatibility. I think it should instead just be part of the first
one (unless you figure a way to introduce it before, which I doubt)
and you can simply append the commit message to the first one explaining
this specific case.

A few nits below:

> diff --git a/src/sock_inet.c b/src/sock_inet.c
> index 028ffaa68..9f10c5654 100644
> --- a/src/sock_inet.c
> +++ b/src/sock_inet.c
> @@ -77,6 +77,10 @@ int sock_inet6_v6only_default = 0;
>  int sock_inet_tcp_maxseg_default = -1;
>  int sock_inet6_tcp_maxseg_default = -1;
>  
> +/* Default MPTCPv4/MPTCPv6 MSS settings. -1=unknown. */
> +int sock_inet_mptcp_maxseg_default = -1;
> +int sock_inet6_mptcp_maxseg_default = -1;
> +

Maybe these should be conditioned to the use of MPTCP (just saving 8
bytes but...).

>  /* Compares two AF_INET sockaddr addresses. Returns 0 if they match or 
> non-zero
>   * if they do not match.
>   */
> @@ -494,6 +498,30 @@ static void sock_inet_prepare()
>  #endif
>   close(fd);
>   }
> +
> +#ifdef __linux__

Here I think a short comment is deserved to explain why __linux__, because
it's the same choice that was made in proto_tcp.c.

Or better, you could probably do that in compat.h:

#ifdef __linux__
# define HA_HAVE_MPTCP 1
# ifndef IPPROTO_TCP
#  define IPPROTO_TCP ...
# endif
#endif

and use #ifdef HA_HAVE_MPTCP here and around the variables above. That
would make it quite clear about what we're really checking for here.

Thanks!
Willy

Re: [PATCH 0/2] Add MPTCP to haproxy

[Willy Tarreau]

Hi Matthieu,

On Fri, Aug 23, 2024 at 04:13:16PM +0200, Matthieu Baerts wrote:
> Hi Willy,
> 
> Thank you for your quick reply!

You're welcome!

> > I'll comment on each patch separately,
> 
> Thank you, please take your time!

That's what I'm doing but I really want to make sure we won't discover
last-minute show-stoppers that require important changes. It's still
possible to do some important ones for a few weeks, so let's tackle all
roadblocks while we can. I really feel like we're on a good track now!

(... mss ...)

> I just filled a new ticket on MPTCP side, not to forget about that:
> 
>   https://github.com/multipath-tcp/mptcp_net-next/issues/515

OK!

> > With that said, from an implementation perspective, it would seem right
> > to make sure that most TCP tunables also work with MPTCP.
> 
> That's what we tried to do. All "common" ones are supported, but it is
> not always easy to define what is "common" and what is not! There are
> many obscure/specific socket options out there :)

I agree :-) For example we're using other horrors such as TCP_REPAIR,
which was initially defined for CRIU, and that we're using to silently
destroy a connection without responding. It's extremely effective against
HTTP botnets as they keep their connection pending and do not consume
anything locally. I don't know if you have it, but if you're interested
in giving it a try, I can help you set it up in haproxy for a test.

Cheers,
Willy

Re: [PATCH 1/2] FEATURE: add MPTCP per address support

[Willy Tarreau]

On Fri, Aug 23, 2024 at 03:34:09PM +0200, Aperence wrote:
(...)
> MPTCP is both supported for the frontend and backend sides.

Great!

> Also added an example of configuration using mptcp along with a backend
> allowing to experiment with it.

Thanks for thinking about testing ;-)

> Note that this is a re-implementation of Björn's work from 3 years ago 
> [4], when haproxy's internals were probably less ready to deal with 
> this, causing his work to be left pending for a while.

Thanks for mentioning his initial work!

> diff --git a/include/haproxy/compat.h b/include/haproxy/compat.h
> index 3829060b7..2347d62cb 100644
> --- a/include/haproxy/compat.h
> +++ b/include/haproxy/compat.h
> @@ -317,6 +317,11 @@ typedef struct { } empty_t;
>  #define queue _queue
>  #endif
>  
> +/* only Linux defines IPPROTO_MPTCP */
> +#ifndef IPPROTO_MPTCP
> +#define IPPROTO_MPTCP 262
> +#endif

Stupid open question: do we really want to define it for other
OSes ? I really don't care, to be honest.

> diff --git a/src/backend.c b/src/backend.c
> index 6956d9bfe..6b865768e 100644
> --- a/src/backend.c
> +++ b/src/backend.c
> @@ -1690,8 +1690,15 @@ int connect_server(struct stream *s)
>  
>   if (!srv_conn->xprt) {
>   /* set the correct protocol on the output stream connector */
> + int mptcp = 0;
> +
> + /* cli_conn can be NULL when the origin of the stream isn't a 
> +  * connection, there's no reason to use MPTCP in this case */
> + if (cli_conn && cli_conn->ctrl) 
> + mptcp = cli_conn->ctrl->sock_prot == IPPROTO_MPTCP;

I don't understand this part, it seems to imply a relation between the
client and the server. I'm not seeing any case where this could be valid
since the config of the client and the server are two totally independent
things.

I'm suspecting that the difficulty here is to know that the server's
address was configured as MPTCP and that since you couldn't find where
to store that piece of info, instead it's inherited from the client. But
that cannot be right as it's really a proxy and there's no relation
between the protocols used on each side. For example it's valid and
normal to have QUIC on the front and HTTP/2 over TCP (or MPTCP soon)
on the back.

If that's the issue, I suspect that we'll have to store the info in
the struct server itself that we want to use mptcp. BTW, based on
the extension you brought to protocol_lookup(), maybe we can have a
more generic field like "alt_proto" or something like this that is
passed directly to protocol_lookup(). This info needs to be
retrieved from the address parser called by _srv_parse_init(). I'm
seeing nowhere to store the "alt" value, and that might be an
indication that str2sa_range() needs to be extended to take yet
another arg to return that one, which you're already setting to
!!mptcp during the lookup.

As such I would suggest the following:
  - a preliminary patch extending str2sa_range() to add an int *alt
before the last "opts" parameter, and that is filled with the
value passed to protocol_lookup() during the function if the
pointer is not NULL. You can put in that patch the change of
definition of protocol_lookup() to use alt instead of dgram BTW.

  - a second (small) patch adding a field alt_proto to the "server"
struct (probably close to net_ns, xprt etc). This field would then
be filled in _srv_parse_init() from the "alt" argument returned by
str2sa_range().

  - then this current patch becomes the third and makes use of srv->alt
instead of !!mptcp below, for the protocol lookup:

>   if (srv) {
> - if (conn_prepare(srv_conn, 
> protocol_lookup(srv_conn->dst->ss_family, PROTO_TYPE_STREAM, 0), srv->xprt)) {
> + if (conn_prepare(srv_conn, 
> protocol_lookup(srv_conn->dst->ss_family, PROTO_TYPE_STREAM, !!mptcp), 
> srv->xprt)) {
(...)

> diff --git a/src/proto_tcp.c b/src/proto_tcp.c
> index d6552b2f1..9cabae11b 100644
> --- a/src/proto_tcp.c
> +++ b/src/proto_tcp.c
> @@ -149,6 +149,102 @@ struct protocol proto_tcpv6 = {
>  
>  INITCALL1(STG_REGISTER, protocol_register, &proto_tcpv6);
>  
> +#ifdef __linux__
> +/* Most fields are copied from proto_tcpv4 */
> +struct protocol proto_mptcpv4 = {
> + .name   = "mptcpv4",
(...)
> + .receivers  = LIST_HEAD_INIT(proto_mptcpv4.receivers),
> + .nb_receivers   = 0,

Thanks to the very latest changes in 3.1-dev6 that you rebased on, you
can drop these two lines, they're now initialized by protocol_register().
I figured I got trapped 3 times in less than one hour, it w

Re: [PATCH 0/2] Add MPTCP to haproxy

[Matthieu Baerts]

Hi Willy,

Thank you for your quick reply!

On 23/08/2024 15:58, Willy Tarreau wrote:
> On Fri, Aug 23, 2024 at 03:34:08PM +0200, Aperence wrote:

(...)

> I'll comment on each patch separately,

Thank you, please take your time!

> though I'll respond to the
> question below:
> 
>>   - Patch 2:
>> - Fix a warning about TCP_MAXSEG not being supported while MPTCP is 
>> used by declaring new constants sock_inet(6)_mptcp_maxseg_default.
>> Those constants represent the MSS supported by MPTCP (== -1 for the
>> moment, as long as TCP_MAXSEG is not supported). For the moment, 
>> MPTCP doesn't support TCP_MAXSEG socket option, mainly because it 
>> has apparently never been requested before, apparently. It should 
>> not be difficult to implement it, but is it an important option
>> for HAProxy?
> 
> It's not that common anymore but 15 years ago when ADSL over PPPoE was
> everywhere, it was super frequent to see users not able to upload large
> POST data over TCP because their PC connected with a 1500 MTU to their
> PPPoE modem/router was seeing its packets rejected due to the extra 8
> bytes of PPPoE encapsulation. Many people had by then implemented the
> set-mss rule in iptables on the server infrastructure to deal with this,
> but high-bandwidth hosting providers couldn't even just place a firewall
> in front of haproxy just to deal with that, so in this case it was very
> convenient to permit haproxy to simply configure the socket to negotiate
> a lower MSS compatible with everyone's setup.
> 
> Nowadays with everyone having a triple-play box provided by the ISP,
> all this stuff is transparent to the user (when needed at all). However
> I've been aware of users doing that as well to fixup trouble caused by
> inter-company VPN links. You know, the type of places where everyone
> says "it's right on my side, it must be the other side"...
> 
> So I guess that nowadays the mss setting mostly serves to fixup VPN
> environments, and it's not much likely that we'll see that with MPTCP
> soon I think (but I could of course be wrong).

Many thanks for this explanation!

I just filled a new ticket on MPTCP side, not to forget about that:

  https://github.com/multipath-tcp/mptcp_net-next/issues/515

> With that said, from an implementation perspective, it would seem right
> to make sure that most TCP tunables also work with MPTCP.

That's what we tried to do. All "common" ones are supported, but it is
not always easy to define what is "common" and what is not! There are
many obscure/specific socket options out there :)

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

Re: [PATCH 0/2] Add MPTCP to haproxy

[Willy Tarreau]

Hello!

On Fri, Aug 23, 2024 at 03:34:08PM +0200, Aperence wrote:
> Multipath TCP (MPTCP), standardized in RFC8684 [1], is a TCP extension
> that enables a TCP connection to use different paths.
> 
> Multipath TCP has been used for several use cases. On smartphones, MPTCP
> enables seamless handovers between cellular and Wi-Fi networks while
> preserving established connections. This use-case is what pushed Apple
> to use MPTCP since 2013 in multiple applications [2]. On dual-stack
> hosts, Multipath TCP enables the TCP connection to automatically use the
> best performing path, either IPv4 or IPv6. If one path fails, MPTCP
> automatically uses the other path.
> 
> To benefit from MPTCP, both the client and the server have to support
> it. Multipath TCP is a backward-compatible TCP extension that is enabled
> by default on recent Linux distributions (Debian, Ubuntu, Redhat, ...).
> Multipath TCP is included in the Linux kernel since version 5.6 [3]. To
> use it on Linux, an application must explicitly enable it when creating
> the socket. No need to change anything else in the application.
> 
> These patches are a continuation of the work [4] realized by Dorian 
> Craps and Matthieu Baerts.

It's great to see Dorian's work pursued, thank you for this series!

I'll comment on each patch separately, though I'll respond to the
question below:

>   - Patch 2:
> - Fix a warning about TCP_MAXSEG not being supported while MPTCP is 
> used by declaring new constants sock_inet(6)_mptcp_maxseg_default.
> Those constants represent the MSS supported by MPTCP (== -1 for the
> moment, as long as TCP_MAXSEG is not supported). For the moment, 
> MPTCP doesn't support TCP_MAXSEG socket option, mainly because it 
> has apparently never been requested before, apparently. It should 
> not be difficult to implement it, but is it an important option
> for HAProxy?

It's not that common anymore but 15 years ago when ADSL over PPPoE was
everywhere, it was super frequent to see users not able to upload large
POST data over TCP because their PC connected with a 1500 MTU to their
PPPoE modem/router was seeing its packets rejected due to the extra 8
bytes of PPPoE encapsulation. Many people had by then implemented the
set-mss rule in iptables on the server infrastructure to deal with this,
but high-bandwidth hosting providers couldn't even just place a firewall
in front of haproxy just to deal with that, so in this case it was very
convenient to permit haproxy to simply configure the socket to negotiate
a lower MSS compatible with everyone's setup.

Nowadays with everyone having a triple-play box provided by the ISP,
all this stuff is transparent to the user (when needed at all). However
I've been aware of users doing that as well to fixup trouble caused by
inter-company VPN links. You know, the type of places where everyone
says "it's right on my side, it must be the other side"...

So I guess that nowadays the mss setting mostly serves to fixup VPN
environments, and it's not much likely that we'll see that with MPTCP
soon I think (but I could of course be wrong).

With that said, from an implementation perspective, it would seem right
to make sure that most TCP tunables also work with MPTCP.

Thanks!
Willy

Re: minor patch to add environment variables for http and tcp clf log formats

[Willy Tarreau]

Hi Nathan,

On Tue, Aug 20, 2024 at 04:19:35PM +, Nathan Wehrman wrote:
> Since we just implemented the TCP CLF format I thought it would be a quick
> and easy time to add a couple of new environment variables.  It's a
> very small and simple patch but if a person needed this and it wasn't
> available then it's very frustrating so I thought I'd just get it done.

Thank you!

> If I missed
> anything then please let me know but I think it's just these 4 new lines.

There were three minor issues that I corrected:

> From 7cc48e41c54c6a59da62362f576397b46d09ec14 Mon Sep 17 00:00:00 2001
> From: Nathan Wehrman 
> Date: Tue, 20 Aug 2024 09:11:34 -0700
> Subject: MINOR: Created env variables for http and tcp clf formats

Usually we place a subsystem name here so that it's easier to figure
what this is about when reviewing long series of commits (typically
during backports or bisect sessions), it gives a bit of context and
usually helps the reader spot something without having to entirely
read hundreds of lines. There's nothing strict at all, you can see
stuff like "h2", "mworker" or "quic" for example. Usually what's purely
related to the config is tagged with "config" or "cfgparse" when it's
more specific to the parser. Here I'll add "config" since it's mostly
something that improves the config abilities.

> --- a/doc/configuration.txt
> +++ b/doc/configuration.txt
> @@ -927,6 +927,10 @@ file, or could be inherited by a program (See 3.7. 
> Programs):
>defined in section 8.2.3 "HTTP log format". It can be used to override the
>default log format without having to copy the whole original definition.
>  
> +* HAPROXY_HTTP_CLF_LOG_FMT: contains the value of the default HTTP CLF log 
> format as
 
^^
> +  defined in section 8.2.3 "HTTP log format". It can be used to override the
> +  default log format without having to copy the whole original definition.

For the doc we wap the lines at 80 chars so that they render correctly in
default window sizes. I've re-justified the 3 lines.

> --- a/src/haproxy.c
> +++ b/src/haproxy.c
> @@ -1188,8 +1188,10 @@ static int read_cfg(char *progname)
>* unset them after the list_for_each_entry loop.
>*/
>   setenv("HAPROXY_HTTP_LOG_FMT", default_http_log_format, 1);
> - setenv("HAPROXY_HTTPS_LOG_FMT", default_https_log_format, 1);
> + setenv("HAPROXY_HTTP_CLF_LOG_FMT", clf_http_log_format, 1);
> +setenv("HAPROXY_HTTPS_LOG_FMT", default_https_log_format, 1);
>   setenv("HAPROXY_TCP_LOG_FMT", default_tcp_log_format, 1);
> + setenv("HAPROXY_TCP_CLF_LOG_FMT", clf_tcp_log_format, 1);

You seem to be having an issue with your editor's tab handling, because
there was a similar one in your previous patch. The line about
"HAPROXY_HTTPS_LOG_FMT" had its leading tab replaced with 4 spaces. I
suspect you're viewing with a tab size of 4 and maybe you manually fixed
a mistake on the wrong line and the editor turned your tabs to spaces.

It's worth checking the output of "git diff" to avoid such things. Also
if you add:

   [color]
ui = true

To your global git config (e.g. "git config color.ui true" or
"git config --global color.ui true"), git diff will then also highlight
trailing spaces/tabs and such annoying changes on invisible chars.

Now applied, thank you!
willy

Re: [PATCH] BUG/MINOR: stats-html: improve markup and fix css in dark mode

[Willy Tarreau]

On Tue, Aug 20, 2024 at 03:29:04PM +0200, Nicolas CARPi wrote:
> Hi Willy,
> 
> On 20 Aug, Willy Tarreau wrote:
> > Normally it's preferable to make one commit per functional change. 
> Noted. Here are three patches then. I've taken care of explaining the 
> reasoning behind each of these changes in the commit messages, which 
> should answer your interrogations! :)

Perfect now, all applied, thank you!

Willy

Re: [PATCH] BUG/MINOR: stats-html: improve markup and fix css in dark mode

[Nicolas CARPi]

Hi Willy,

On 20 Aug, Willy Tarreau wrote:
> Normally it's preferable to make one commit per functional change. 
Noted. Here are three patches then. I've taken care of explaining the 
reasoning behind each of these changes in the commit messages, which 
should answer your interrogations! :)

Cheers,
~Nicolas
>From c3f0cb4be4d5802cc3e63d4501c17be1b1392ca1 Mon Sep 17 00:00:00 2001
From: Nicolas CARPi 
Date: Tue, 20 Aug 2024 15:20:10 +0200
Subject: [PATCH 3/3] BUG/MINOR: stats: add lang attribute to html tag

The "html" element of the stats page was missing a "lang" attribute.
This change specifies the "en" value, which corresponds to english
language.

It is also a required element for WCAG Success Criterion 3.1.1, which
renders the web more accessible through a set of requirements. In this
case it allows assistive technologies such as screen readers to
determine the language of the page.

MDN page: 
https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/lang
HTML standard: https://html.spec.whatwg.org/multipage/dom.html#attr-lang
WCAG criterion: 
https://www.w3.org/WAI/WCAG22/Understanding/language-of-page.html
---
 src/stats-html.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/stats-html.c b/src/stats-html.c
index c69fb9d2a..23cfb91a0 100644
--- a/src/stats-html.c
+++ b/src/stats-html.c
@@ -60,7 +60,7 @@ void stats_dump_html_head(struct appctx *appctx)
/* WARNING! This must fit in the first buffer !!! */
chunk_appendf(chk,
  "\n"
- "Statistics Report for " PRODUCT_NAME 
"%s%s\n"
+ "Statistics Report for " 
PRODUCT_NAME "%s%s\n"
  "\n"
  "\n"
  "

Re: [PATCH] BUG/MINOR: stats-html: improve markup and fix css in dark mode

[Willy Tarreau]

Hi Nicolas,

On Tue, Aug 20, 2024 at 11:08:33AM +0200, Nicolas CARPi wrote:
> From 96f5e7951995be8216ecee81968b0f2c7fe0141a Mon Sep 17 00:00:00 2001
> From: Nicolas CARPi 
> Date: Tue, 20 Aug 2024 10:39:17 +0200
> Subject: [PATCH] BUG/MINOR: stats-html: improve markup and fix css in dark
>  mode
> 
> This patch concerns the HTML stats page:
> * Update the Doctype to a modern one
> * Add the "lang" attribute to the "html" tag. This is required by WCAG
>   Success Criterion 3.1.1 (accessibility standard)
> * Fix the text color in dark mode for the Scope input field

Normally it's preferable to make one commit per functional change. Even
if these ones touch the same thing, they seem to change totally unrelated
stuff (doctype, lang, color preference).

Also what's the motivation/impacts of the first one? Does it render
better or avoid a warning on some clients, is it just because it's cleaner,
etc ? In this case maybe that one should just be tagged cleanup and the
other ones minor. That's low importance, but I prefer that we don't start
mixing unrelated changes in a single commit (and some might argue for
backporting some of them, e.g. the color or lang).

Thanks!
Willy

Re: New option tcplog clf format

[Willy Tarreau]

Hi Nathan,

On Tue, Aug 13, 2024 at 06:34:00PM +, Nathan Wehrman wrote:
> 
> Hello,
> 
> For your consideration I wrote and tested a new a logging format that will
> allow tcp mode proxies to send logs that will adhere to clf (common log
> format).
> This will allow the sending of that data to servers that expect that format
> so it can be parsed but the connections will stand out as TCP.
> To accomplish that I used this log-format string:
> 
> log-format "%{+Q}o %{-Q}ci - - [%T] \"TCP \" 000 %B \"\" \"\" %cp %ms %ft %b
> %s %Th %Tw %Tc %Tt %U %ts-- %ac %fc %bc %sc %rc %sq %bq \"\" \"\" "
> 
> The HTTP request has been replaced with "TCP " and the response code has
> been hard coded to "000".
> 
> I also added a couple of very minor spacing changes.
> 
> The patch file is attached.

Looks good overall, and even if I don't use CLF myself, I obviously have
no objection to this minor change. I fixed two tiny ident issues (one on
a variable declaration and one in a condition) and changed the tag from
MEDIUM to MINOR since it really has no theoretical impact on existing
features, and I've applied it.

I've been wondering why we don't have the CLF equivalent variables for
tcp/http logs (e.g. HAPROXY_HTTP_LOG_FMT), and I suspect that it's simply
because they're less often used. I don't know if there could be any
interest in having the equivalent ones declared for CLF (http/tcp) or
not. Since this is only pure convenience, it's always difficult to guess
whether some users would really benefit from this.

Thanks!
Willy

Re: [PATCH] DOC: fix incorrect english in lua.txt

[Willy Tarreau]

Hi Nicolas,

On Wed, Aug 14, 2024 at 11:16:31AM +0200, Nicolas CARPi wrote:
> From f9cff910630851658a9f126caf1009e08dec Mon Sep 17 00:00:00 2001
> From: Nicolas CARPi 
> Date: Tue, 13 Aug 2024 22:57:56 +0200
> Subject: [PATCH] DOC: fix incorrect english in lua.txt
> 
> This commit fixes some typos, grammatical errors and unusual english
> such as "can not" instead of preferred "cannot".

Interesting, we had a fix a few years ago doing exactly the opposite,
changing "cannot" to "can not" which was supposed to be better English.
With that said we have more "cannot" than "can not" (and I personally
prefer that one for being more common).

I'm applying your patch, thank you!
Willy

Re: [PATCH 1/3] CI: QUIC Interop LibreSSL: document chacha20 test status

[Willy Tarreau]

On Tue, Aug 13, 2024 at 09:11:28PM +0200, Ilia Shipitsin wrote:
> due to https://github.com/haproxy/haproxy/issues/2569 chacha20 is
> disabled completely on LibreSSL. let's add a comment to not forget
> enabling it
(...)

series applied, thanks Ilya!

Willy

Re: [PATCH] FEATURE/MAJOR: Add upstream-proxy-tunnel feature

[Aleksandar Lazic]

Hi Willy.

On 2024-08-12 (Mo.) 16:49, Willy Tarreau wrote:

Hi Alex,

On Mon, Aug 12, 2024 at 11:46:37AM +0200, Aleksandar Lazic wrote:

On Thu, Jun 13, 2024 at 03:00:59AM +0200, Aleksandar Lazic wrote:

The final idea is something like this.

```
tcp-request content upstream-proxy-header Host %[req.ssl_sni,lower]
tcp-request content upstream-proxy-header "$AUTH" "$TOKEN"
tcp-request content upstream-proxy-header Proxy-Connection Keep-Alive
tcp-request content upstream-proxy-target %[req.ssl_sni,lower]

server stream 0.0.0.0:0 upstream-proxy-tunnel
%[req.ssl_sni,lower,map_str(targets.map)]
```

The targets.map should have something like this.
#dest proxy
sni01 proxy01
sni02 proxy02

I hope the background of upstream-proxy-target is now more clear.


- In `server https_Via_Proxy1 0.0.0.0:0 upstream-proxy-tunnel 127.0.0.1:3128`
     from your config, what is 0.0.0.0:0 used for here? This binds to all IPv4
     but on a random free port?


This is required when the destination should be dynamic, it's documented here.
http://docs.haproxy.org/3.0/configuration.html#4.4-do-resolve


A+
Dave


Regards
Alex


Overall I find that there are plenty of good points in this patch and
the discussion around it. But it also raises questions:

- IMHO the server's address should definitely be the proxy's address.
  This will permit to use all the standard mechanisms we have such as
  DNS resolution and health checks and continues to work as a regular
  handshake. I'm not sure this is the case in the current state of the
  patch since I'm seeing an extra "upstream-proxy-tunnel" parameter
  (actually I'm confused by what you call a "target" here, as used in
  upstream-proxy-target).


Well I thought the same but that's then difficult to separate between the
proxy server and the destination server. Maybe we need some kind of server
type?

As you can see in the sequence diagram are both servers from HAProxy point of 
view.
https://github.com/git001/tls-proxy-tunnel/blob/main/README.md#sequence-diagram

The "target" is the "destination server" which is from my point of view the
"server" in haproxy.

The "upstream-proxy-target" is the upstream proxy server. I'm also not happy
with that name and open for suggestions.


Then I'm confused by what the server's configured address becomes.


Fully understand.
Let me separate the word "server" here.

The server in the server line is the final target/destination server which is 
the one which the user want to reach.


The upstream proxy server, similar to the socks4 server, is just a hop server to 
connect to the final target/destination server.



I mean, there are three possibilities:

   1) the ip:port of the server designates the target server, in which
  case only ip:port are passed to the proxy and we never send an FQDN
  there. Then the proxy has to be hard-coded in an option (apparently
  the upstream-proxy-tunnel here). Haproxy then focuses only on the
  target server for load balancing, stickiness, health checks etc and
  always reaches that server through the hard-coded upstream proxy.
  This could match what an application-oriente LB would look for, i.e.
  being able to reach its "local" servers through a proxy. It's just
  that I would be surprised that such a use case really exists, because
  load-balancing remote servers across a single local hard-coded proxy
  doesn't feel natural at all. E.g. the LB is not even on the LAN that
  knows all the remote hosts so such a config is particularly difficult
  to maintain in a working state.

   2) the fqdn:port of the server designates the target server, in which
  case only fqdn:port are passed to the proxy and we let the proxy
  resolve the target server. The rest is the same as above, it's just
  a small variation, but I'm still having a hard time figuring what
  use case that could match.


This is the current implementation of the patch.
The Proxy is the one which then decides if the connection to the 
target/destination server is allowed or not.



   3) the ip:port of the server designates the upstream proxy, and the
  connection's destination is passed to the CONNECT method. In this
  case it means that LB, checks, stickiness etc applies to the proxies
  and not to the servers. In this case we need a distinct field to set
  the target host name. That's more or less what you did with the
  tcp-request upstream-proxy-target rule, which could possible have a
  default on the server line. We could also imagine that a set-dst
  action would allow to force the target destination address, but that
  would be a detail. This more is more about infrastructure than remote
  app servers: you tell the proxy how to reach external services. I've
  seen this requested a few times by those saying "I would like to use
  the proxy protocol from my servers, that would connect to haproxy
  which would then

Re: Tuning HTTP/2 window size

[Willy Tarreau]

Hi Max,

On Wed, Aug 14, 2024 at 06:21:39AM +, Moehl, Maximilian wrote:
> Hi Willy,
> 
> > > Is there a similar mechanism in HAProxy? So far I can only see the
> > > static option for the initial window size which comes with the mentioned
> > > drawbacks.
> >
> > There is nothing similar. One of the problems H2 is facing is that there
> > can be application congestion anywhere in the chain. For example, let's
> > say a POST request is sent to a server subject to a maxconn and remains
> > in the queue for a few seconds. We definitely don't want to block the
> > whole connection during this time because we've oversized the window.
> >
> > And there's also the problem of not allocating too many buffers to each
> > stream, or it becomes a trivial DoS.
> 
> That makes sense, since we had to accommodate the use-case which triggered the
> investigation we increased the window size to 512k as a middle-ground. We 
> don't
> usually see congestion on our HAProxies so we are hoping that this does not
> cause any other issues, we'll see once it hits prod.

There used to be a case where it was causing significant problems: initially
this initial window size was both for frontend and backends. If you were using
H2 on the backend as well, and two requests from different clients were sent
over the same H2 backend connection, a slow reader would be sufficient to
cause the other one to experience read timeouts. Now we've addressed this
using two points:
  - it's possible to configure the backend-side window size separately
from the frontend-side
  - "http-reuse safe" (the default mode) makes H2 backend connections
private, that is, they're not shared between multiple client
connections. This results in more connections on the backend side
but the progress on the backend matches the progress on the frontend.

The remaining issues that you may encounter with large frontend windows
is if a server sometimes pauses when consuming POST bodies, it may
occasionally prevent the client from performing requests in parallel
over the same connection. That's quite rare to meet such trouble of
course, but we want to stay on the safe side by default, which is why
we don't use a larger window by default. If you know that your servers
are "normal" and consume the bodies sent to them, there's no real
problem with increasing the window size.

A case I met a long time ago that could match a pathologic one was a
few URLs served by a partner's server behind a slow leased line. In
this case, a large POST sent over this link could occasionally face
congestion and could temporarily freeze the client connection for
the time it took to upload a large body. As you see it's not everyone
that hosts such applications.

Hoping this helps,
Willy

Re: [PATCH] DOC: fix incorrect english in lua.txt

[Nicolas CARPi]

Sending it again because it seems it never reached the mailing list for 
some reason!?

On 13 Aug, Nicolas CARPi wrote:
> Dear maintainers,
> 
> Please find attached a patch for a few mistakes I found in doc/lua.txt 
> file.
> 
> Best,
> ~Nicolas

>From f9cff910630851658a9f126caf1009e08dec Mon Sep 17 00:00:00 2001
From: Nicolas CARPi 
Date: Tue, 13 Aug 2024 22:57:56 +0200
Subject: [PATCH] DOC: fix incorrect english in lua.txt

This commit fixes some typos, grammatical errors and unusual english
such as "can not" instead of preferred "cannot".
---
 doc/lua.txt | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/doc/lua.txt b/doc/lua.txt
index 5d41a30cf..9440dc83b 100644
--- a/doc/lua.txt
+++ b/doc/lua.txt
@@ -24,8 +24,8 @@ However, this can be read by Lua beginners. Some examples are 
detailed.
 Why a scripting language in HAProxy
 ===
 
-HAProxy 1.5 makes at possible to do many things using samples, but some people
-want to more combining results of samples fetches, programming conditions and
+HAProxy 1.5 makes it possible to do many things using samples, but some people
+want to more by combining results of samples fetches, programming conditions 
and
 loops which is not possible. Sometimes people implement these functionalities
 in patches which have no meaning outside their network. These people must
 maintain these patches, or worse we must integrate them in the HAProxy
@@ -135,7 +135,7 @@ these functions:
 These functions are the execution entry points.
 
 HTTP action must be used for manipulating HTTP request headers. This action
-can not manipulates HTTP content. It is dangerous to use the channel
+cannot manipulates HTTP content. It is dangerous to use the channel
 manipulation object with an HTTP request in an HTTP action. The channel
 manipulation can transform a valid request in an invalid request. In this case,
 the action will never resume and the processing will be frozen. HAProxy
@@ -155,7 +155,7 @@ with only one stream, HAProxy seems to run fine. When the 
code is used with
 production stream, HAProxy encounters some slow processing, and it cannot
 hold the load.
 
-However, during the initialisation state, you can obviously using blocking
+However, during the initialisation state, you can obviously use blocking
 functions. There are typically used for loading files.
 
 The list of prohibited standard Lua functions during the runtime contains all
@@ -219,7 +219,7 @@ I executed a Lua loop between 10 seconds with different 
values for the
 10| 710
 100   | 710
 
-The result showed that from 9000 instructions between two interrupt, we reached
+The result showed that from 9000 instructions between two interrupts, we 
reached
 a ceil, so the default parameter is 10 000.
 
 When HAProxy interrupts the Lua processing, we have two states possible:
@@ -524,9 +524,9 @@ server, and processing the many errors which can occurs 
during these exchanges.
 HAProxy is not designed for having a third connection established to a third
 party server.
 
-The solution consist to put the main stream in pause waiting for the end of the
-exchanges with the third connection. This is completed by a signal between
-internal tasks. The following graph shows the HAProxy Lua socket:
+The solution consists of putting the main stream in pause, waiting for the end
+of the exchanges with the third connection. This is completed by a signal
+between internal tasks. The following graph shows the HAProxy Lua socket:
 
 
   ++
@@ -615,7 +615,7 @@ Start point
 ---
 
 The HAProxy global directive "lua-load " allows to load an Lua file. This
-is the entry point. This load become during the configuration parsing, and the
+is the entry point. This load occurs during the configuration parsing, and the
 Lua file is immediately executed.
 
 All the register_*() functions must be called at this time because they are 
used
-- 
2.46.0

Re: Tuning HTTP/2 window size

[Moehl, Maximilian]

Hi Willy,

> > Is there a similar mechanism in HAProxy? So far I can only see the
> > static option for the initial window size which comes with the mentioned
> > drawbacks.
>
> There is nothing similar. One of the problems H2 is facing is that there
> can be application congestion anywhere in the chain. For example, let's
> say a POST request is sent to a server subject to a maxconn and remains
> in the queue for a few seconds. We definitely don't want to block the
> whole connection during this time because we've oversized the window.
>
> And there's also the problem of not allocating too many buffers to each
> stream, or it becomes a trivial DoS.

That makes sense, since we had to accommodate the use-case which triggered the
investigation we increased the window size to 512k as a middle-ground. We don't
usually see congestion on our HAProxies so we are hoping that this does not
cause any other issues, we'll see once it hits prod.

> However, as I mentioned somewhere (maybe the issue above, I don't remember),
> I think that the vast majority of users are not downloading and uploading at
> the same time over a same connection. Either they're uploading a large file
> or downloading contents to be rendered.

I agree, that seems to be the case for our scenarios.

> That's something to think about. Thanks for re-heating this old topic!

Thanks for taking the time to look into it!

Regards
Max

Re: Company to set up your product

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

You've reached open-source mailing list.

please clarify what product do you mean, open source HAProxy ?

if some of commercial product, maybe the best place would be Contact
HAProxy Technologies 

вт, 13 авг. 2024 г. в 10:18, Christopher Doerr :

> Hello,
>
> We are looking for a company that can set up your product in Germany and
> can also provide us with support. Therefore my question is do you have a
> recommendation which company can do this for us?
>
> Many thanks and best regards,
> Christopher Dörr
>
> Christopher Doerr
> Junior Engineer IT Enablement
> IT & Information Services
>
>
> T: +49 6103 402-116
> M: +49 151 74495029
> doerr.christop...@smc.com
> http://www.smc.de
>
>
>
> [image: FirstClimate_Label_CO2-Compensation_Scope_QR_DE.jpg]
> 
>
> SMC Deutschland GmbH | Boschring 13-15 | 63329 Egelsbach | Germany
> Amtsgericht Offenbach | District Court Offenbach HRB 31580
> Geschäftsführer | Managing Director: Daniel Langmeier
> Datenschutzhinweis  | Disclaimer
> 
>
>
>
>

Re: Tuning HTTP/2 window size

[Willy Tarreau]

Hi Maximilian,

sorry for the delay, I'm sure I noticed the subject but likely archived
the message before reading it. Thanks to Lukas for pinging again about
it in GH issue #352 which was already about this!

On Tue, Jul 23, 2024 at 08:38:44AM +, Moehl, Maximilian wrote:
> We've received reports from users that are struggling to upload large
> files via our HAProxies. With some testing, we discovered that h1 is
> relatively stable as latency increases while h2 constantly degrades.
> We've narrowed the issue down to `tune.h2.fe.initial-window-size` which
> states:
> 
> > The default value of 65536 allows up to 5 Mbps of bandwidth per client
> > over a 100 ms ping time, and 500 Mbps for 1 ms ping time.
> 
> Which is in line with our experiments.
> 
> While researching the topic, I've come across a blog post from
> Cloudflare [1] where they mention improvements to their edge which
> dynamically adjusts the window size to accommodate the latency and
> bandwidth constraints. They've upstreamed the patch to nginx [2]
> (although I must admit that I'm not sure whether the change is fixing a
> nginx specific issue or the general issue we are experiencing here).
> 
> Is there a similar mechanism in HAProxy? So far I can only see the
> static option for the initial window size which comes with the mentioned
> drawbacks.

There is nothing similar. One of the problems H2 is facing is that there
can be application congestion anywhere in the chain. For example, let's
say a POST request is sent to a server subject to a maxconn and remains
in the queue for a few seconds. We definitely don't want to block the
whole connection during this time because we've oversized the window.

And there's also the problem of not allocating too many buffers to each
stream, or it becomes a trivial DoS.

However, as I mentioned somewhere (maybe the issue above, I don't remember),
I think that the vast majority of users are not downloading and uploading at
the same time over a same connection. Either they're uploading a large file
or downloading contents to be rendered. Maybe in this case we could consider
that the allocatable buffers for the mux could in fact be repurposed for any
stream. We already have one per-stream buffer. If the stream getting the
POST was able to allocate more buffers, it could then release the connection
from pending frames and allow the client to use a larger send window. One of
the difficulties is to figure how to allocate them but we can afford a few
round trips during a large upload, so actually the stream itself could
increase the window when allocating new buffers. In our case, buffers were
really not meant to be extensible on the rx side, but I suspect it might
not be too hard to do.

I'll also need to have a look at what Cloudflare did for NGINX, they might
have updated that based on their observations and corner cases.

That's something to think about. Thanks for re-heating this old topic!

Willy

Re: minor correction to the configuration manual

[Willy Tarreau]

Hi Nathan,

On Tue, Aug 13, 2024 at 05:45:37PM +, Nathan Wehrman wrote:
> The configuration manual currently lists "option tcplog" as valid for use in
> a backend.
> This is not correct. This patch simply fixes that one line.

Thank you, you're right, now merged.

Please check your git setup, I think you have not set the "user.name"
variable and your name didn't appear in the From field, but only the
e-mail:

   From: nwehrman 
   Date: Tue, 13 Aug 2024 10:36:28 -0700
   Subject: MINOR correct the table for option tcplog

Normally this is done with:

   git config --global user.name "Foo Bar"

I've fixed it in the patch and merged it.

Thanks,
Willy

Re: [PATCH] CI: keep logs for failed QIUC Interop jobs

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

ping :)

this will allow to investigate the following failures: QUIC Interop
LibreSSL · haproxy/haproxy@0982bfd · GitHub


ср, 7 авг. 2024 г. в 13:03, Илья Шипицин :

> patch attached.
>
> ср, 7 авг. 2024 г. в 12:34, Илья Шипицин :
>
>> please ignore, I'll send better patch
>>
>> ср, 7 авг. 2024 г. в 12:33, Ilia Shipitsin :
>>
>>> it might be useful to investigate logs of failed tests. to keep
>>> artifacts small the following actions are taken
>>> - only failed logs are kept
>>> - logs retention is 6 days
>>> ---
>>>  .github/workflows/quic-interop-aws-lc.yml   | 13 +
>>>  .github/workflows/quic-interop-libressl.yml | 13 +
>>>  2 files changed, 26 insertions(+)
>>>
>>> diff --git a/.github/workflows/quic-interop-aws-lc.yml
>>> b/.github/workflows/quic-interop-aws-lc.yml
>>> index 1e0220d71..6f1bb38be 100644
>>> --- a/.github/workflows/quic-interop-aws-lc.yml
>>> +++ b/.github/workflows/quic-interop-aws-lc.yml
>>> @@ -75,3 +75,16 @@ jobs:
>>>pip install -r requirements.txt --break-system-packages
>>>python run.py -l logs -r haproxy=ghcr.io/${{
>>>  github.repository }}:aws-lc -t ${{
>>> matrix.suite.tests }} -c ${{ matrix.suite.client }} -s haproxy
>>>
>>> +  - name: Delete succeeded logs
>>> +if: failure()
>>> +run: |
>>> +  cd quic-interop-runner/logs/haproxy_${{ matrix.suite.client }}
>>> +  cat ../../result.json | jq -r '.results[][] |
>>> select(.result=="succeeded") | .name' | xargs rm -rf
>>> +
>>> +  - name: Logs upload
>>> +if: failure()
>>> +uses: actions/upload-artifact@v4
>>> +with:
>>> +  name: logs
>>> +  path: quic-interop-runner/logs/
>>> +  retention-days: 6
>>> \ No newline at end of file
>>> diff --git a/.github/workflows/quic-interop-libressl.yml
>>> b/.github/workflows/quic-interop-libressl.yml
>>> index 2a635bcfa..16850ec35 100644
>>> --- a/.github/workflows/quic-interop-libressl.yml
>>> +++ b/.github/workflows/quic-interop-libressl.yml
>>> @@ -75,3 +75,16 @@ jobs:
>>>pip install -r requirements.txt --break-system-packages
>>>python run.py -l logs -r haproxy=ghcr.io/${{
>>>  github.repository }}:libressl -t ${{
>>> matrix.suite.tests }} -c ${{ matrix.suite.client }} -s haproxy
>>>
>>> +  - name: Delete succeeded logs
>>> +if: failure()
>>> +run: |
>>> +  cd quic-interop-runner/logs/haproxy_${{ matrix.suite.client }}
>>> +  cat ../../result.json | jq -r '.results[][] |
>>> select(.result=="succeeded") | .name' | xargs rm -rf
>>> +
>>> +  - name: Logs upload
>>> +if: failure()
>>> +uses: actions/upload-artifact@v4
>>> +with:
>>> +  name: logs
>>> +  path: quic-interop-runner/logs/
>>> +  retention-days: 6
>>> \ No newline at end of file
>>> --
>>> 2.43.0.windows.1
>>>
>>>

Re: [PATCH] BUG/BUILD: deviceatlas: corrected path for *.cpp file compilation

[David Carlier]

No worries :) thanks for your concern. Cheers.

On Mon, Aug 12, 2024 at 1:04 PM Miroslav Zagorac 
wrote:

> On 12. 08. 2024. 13:50, David Carlier wrote:
> > Hi Miroslav,
> >
> > Am I correct to assume the line you deleted relates to the dummy build
> > version of the DA library ?
> >
> > The sole C++ compilation unit is already taken care of above `(ifeq
> > (DEVICEATLAS_NOCACHE),)`.
> >
> > Regards.
>
> Hello David,
>
> I just replaced the hard-coded path with the one contained in the
> DEVICEATLAS_SRC variable, because for some reason I couldn't compile
> haproxy.
>
> However, now it works and my proposed commit can be ignored.  I apologize.
>
>
> Best regards,
>
> --
> Miroslav Zagorac
> Senior Developer
>

Re: [PATCH] FEATURE/MAJOR: Add upstream-proxy-tunnel feature

[Willy Tarreau]

Hi Alex,

On Mon, Aug 12, 2024 at 11:46:37AM +0200, Aleksandar Lazic wrote:
> > On Thu, Jun 13, 2024 at 03:00:59AM +0200, Aleksandar Lazic wrote:
> > > > The final idea is something like this.
> > > > 
> > > > ```
> > > > tcp-request content upstream-proxy-header Host %[req.ssl_sni,lower]
> > > > tcp-request content upstream-proxy-header "$AUTH" "$TOKEN"
> > > > tcp-request content upstream-proxy-header Proxy-Connection Keep-Alive
> > > > tcp-request content upstream-proxy-target %[req.ssl_sni,lower]
> > > > 
> > > > server stream 0.0.0.0:0 upstream-proxy-tunnel
> > > > %[req.ssl_sni,lower,map_str(targets.map)]
> > > > ```
> > > > 
> > > > The targets.map should have something like this.
> > > > #dest proxy
> > > > sni01 proxy01
> > > > sni02 proxy02
> > > > 
> > > > I hope the background of upstream-proxy-target is now more clear.
> > > > 
> > > > > - In `server https_Via_Proxy1 0.0.0.0:0 upstream-proxy-tunnel 
> > > > > 127.0.0.1:3128`
> > > > >     from your config, what is 0.0.0.0:0 used for here? This binds to 
> > > > > all IPv4
> > > > >     but on a random free port?
> > > > 
> > > > This is required when the destination should be dynamic, it's 
> > > > documented here.
> > > > http://docs.haproxy.org/3.0/configuration.html#4.4-do-resolve
> > > > 
> > > > > A+
> > > > > Dave
> > > > 
> > > > Regards
> > > > Alex
> > 
> > Overall I find that there are plenty of good points in this patch and
> > the discussion around it. But it also raises questions:
> > 
> >- IMHO the server's address should definitely be the proxy's address.
> >  This will permit to use all the standard mechanisms we have such as
> >  DNS resolution and health checks and continues to work as a regular
> >  handshake. I'm not sure this is the case in the current state of the
> >  patch since I'm seeing an extra "upstream-proxy-tunnel" parameter
> >  (actually I'm confused by what you call a "target" here, as used in
> >  upstream-proxy-target).
> 
> Well I thought the same but that's then difficult to separate between the
> proxy server and the destination server. Maybe we need some kind of server
> type?
> 
> As you can see in the sequence diagram are both servers from HAProxy point of 
> view.
> https://github.com/git001/tls-proxy-tunnel/blob/main/README.md#sequence-diagram
> 
> The "target" is the "destination server" which is from my point of view the
> "server" in haproxy.
> 
> The "upstream-proxy-target" is the upstream proxy server. I'm also not happy
> with that name and open for suggestions.

Then I'm confused by what the server's configured address becomes.

I mean, there are three possibilities:

  1) the ip:port of the server designates the target server, in which
 case only ip:port are passed to the proxy and we never send an FQDN
 there. Then the proxy has to be hard-coded in an option (apparently
 the upstream-proxy-tunnel here). Haproxy then focuses only on the
 target server for load balancing, stickiness, health checks etc and
 always reaches that server through the hard-coded upstream proxy.
 This could match what an application-oriente LB would look for, i.e.
 being able to reach its "local" servers through a proxy. It's just
 that I would be surprised that such a use case really exists, because
 load-balancing remote servers across a single local hard-coded proxy
 doesn't feel natural at all. E.g. the LB is not even on the LAN that
 knows all the remote hosts so such a config is particularly difficult
 to maintain in a working state.

  2) the fqdn:port of the server designates the target server, in which
 case only fqdn:port are passed to the proxy and we let the proxy
 resolve the target server. The rest is the same as above, it's just
 a small variation, but I'm still having a hard time figuring what
 use case that could match.

  3) the ip:port of the server designates the upstream proxy, and the
 connection's destination is passed to the CONNECT method. In this
 case it means that LB, checks, stickiness etc applies to the proxies
 and not to the servers. In this case we need a distinct field to set
 the target host name. That's more or less what you did with the
 tcp-request upstream-proxy-target rule, which could possible have a
 default on the server line. We could also imagine that a set-dst
 action would allow to force the target destination address, but that
 would be a detail. This more is more about infrastructure than remote
 app servers: you tell the proxy how to reach external services. I've
 seen this requested a few times by those saying "I would like to use
 the proxy protocol from my servers, that would connect to haproxy
 which would then connect to the forward proxy to reach the
 destination". The choice might be debatable of course but I think it
 does cover a number of situations where haproxy and the forward proxy
 farm are here to provide

Re: [PATCH] BUG/BUILD: deviceatlas: corrected path for *.cpp file compilation

[Miroslav Zagorac]

On 12. 08. 2024. 13:50, David Carlier wrote:
> Hi Miroslav,
> 
> Am I correct to assume the line you deleted relates to the dummy build
> version of the DA library ?
> 
> The sole C++ compilation unit is already taken care of above `(ifeq
> (DEVICEATLAS_NOCACHE),)`.
> 
> Regards.

Hello David,

I just replaced the hard-coded path with the one contained in the
DEVICEATLAS_SRC variable, because for some reason I couldn't compile haproxy.

However, now it works and my proposed commit can be ignored.  I apologize.


Best regards,

-- 
Miroslav Zagorac
Senior Developer

Re: [PATCH] BUG/BUILD: deviceatlas: corrected path for *.cpp file compilation

[David Carlier]

Hi Miroslav,

Am I correct to assume the line you deleted relates to the dummy build
version of the DA library ?

The sole C++ compilation unit is already taken care of above `(ifeq
(DEVICEATLAS_NOCACHE),)`.

Regards.

On Mon, Aug 12, 2024 at 11:35 AM Miroslav Zagorac 
wrote:

> Hello all,
>
> This patch fixes a bug made in commit 9cfd6c8f8e ("BUILD/MEDIUM:
> deviceatlas:
> addon build rework.") where the path to the *.cpp file is hard-coded.
>
>
> Best regards,
>
> --
> Miroslav Zagorac
> Senior Developer

Re: [PATCH] FEATURE/MAJOR: Add upstream-proxy-tunnel feature

[Aleksandar Lazic]

Hi Willy.

On 2024-08-12 (Mo.) 10:01, Willy Tarreau wrote:

Hi Alex,

I finally found time to have a look into this!


Great :-)


On Thu, Jun 13, 2024 at 03:00:59AM +0200, Aleksandar Lazic wrote:

The final idea is something like this.

```
tcp-request content upstream-proxy-header Host %[req.ssl_sni,lower]
tcp-request content upstream-proxy-header "$AUTH" "$TOKEN"
tcp-request content upstream-proxy-header Proxy-Connection Keep-Alive
tcp-request content upstream-proxy-target %[req.ssl_sni,lower]

server stream 0.0.0.0:0 upstream-proxy-tunnel
%[req.ssl_sni,lower,map_str(targets.map)]
```

The targets.map should have something like this.
#dest proxy
sni01 proxy01
sni02 proxy02

I hope the background of upstream-proxy-target is now more clear.


- In `server https_Via_Proxy1 0.0.0.0:0 upstream-proxy-tunnel 127.0.0.1:3128`
    from your config, what is 0.0.0.0:0 used for here? This binds to all IPv4
    but on a random free port?


This is required when the destination should be dynamic, it's documented here.
http://docs.haproxy.org/3.0/configuration.html#4.4-do-resolve


A+
Dave


Regards
Alex


Overall I find that there are plenty of good points in this patch and
the discussion around it. But it also raises questions:

   - IMHO the server's address should definitely be the proxy's address.
 This will permit to use all the standard mechanisms we have such as
 DNS resolution and health checks and continues to work as a regular
 handshake. I'm not sure this is the case in the current state of the
 patch since I'm seeing an extra "upstream-proxy-tunnel" parameter
 (actually I'm confused by what you call a "target" here, as used in
 upstream-proxy-target).


Well I thought the same but that's then difficult to separate between the proxy 
server and the destination server. Maybe we need some kind of server type?


As you can see in the sequence diagram are both servers from HAProxy point of 
view.
https://github.com/git001/tls-proxy-tunnel/blob/main/README.md#sequence-diagram

The "target" is the "destination server" which is from my point of view the 
"server" in haproxy.


The "upstream-proxy-target" is the upstream proxy server. I'm also not happy 
with that name and open for suggestions.



   - for the remote server's name, typically the one we'd extract from
 a Host header usually or from SNI in some cases (maybe that's what
 you called the "target" ?), we'd definitely need to support an
 expression sent as text (usually ip:port), though we could as well
 find it convenient to set the host and the port separately. In this
 case it's likely that the sole presence of this expression could be
 sufficient to enable the feature.

   - the Host field should default to the one in the URI if there's none
 explicitly added.


Full Ack.


   - I like the fact that you implemented support for headers upfront,
 as we all know that this will be a prerequisite for many users.


:-)


   - you should definitely get rid of allthese DPRINTF() debug traces.
 The simple fact that I no longer know how to enable them should ring
 a bell about their relevance ;-)


:-)

What's the suggested alternative?
I thought "*TRACE*" but haven't seen any good documentation how to use it.
I'm happy to switch to any better debugging setup.


   - we need to make sure this handshake is sent before SSL and not after.
 This should permit to do what users normally expect, i.e., encrypt a
 TCP connection and encapsulate it into a CONNECT request.


I thought that's how I added it, looks like I have overseen something.
From my understanding is the call in "xprt_handshake_io_cb(...)" before the 
TLS/SSL handshake, is this wrong?



   - please don't realign the srv_kw_list at the end of the patch, it makes
 it impossible to spot the addition(s) there. We can tolerate a few
 unaligned entries (even though often it's a sign that a shorter name
 would be welcome), and if you really need to realign, please do that
 in a separate patch.


Okay. I'm happy to discuss a better name.


   - I think the headers addition should also be done into a separate patch
 since it's a significant part of the patch. It will also enlighten the
 default behavior since the code must work without extra header rules.


Okay.


   - and BTW there's no need for this "upt" entry in the proxy struct, if
 a string is needed you can just use an ist or even char* which are
 simpler. But I do think that it should be extracted from the request
 (something we need to figure how by default).


In general, is the "struct proxy{ { ... }tcp_req; }" the right one to add this 
feature?

Maybe it's better to add it into the l4 or l5 ruleset?

I haven't seen any good way to define a server type like "upstream", "origin", 
"any-other-type" for the server line maybe this type keyword could help to get 
the separation between an upstream proxy server and an origin/target/destination 
se

Re: [PATCH] FEATURE/MAJOR: Add upstream-proxy-tunnel feature

[Willy Tarreau]

Hi Alex,

I finally found time to have a look into this!

On Thu, Jun 13, 2024 at 03:00:59AM +0200, Aleksandar Lazic wrote:
> > The final idea is something like this.
> > 
> > ```
> > tcp-request content upstream-proxy-header Host %[req.ssl_sni,lower]
> > tcp-request content upstream-proxy-header "$AUTH" "$TOKEN"
> > tcp-request content upstream-proxy-header Proxy-Connection Keep-Alive
> > tcp-request content upstream-proxy-target %[req.ssl_sni,lower]
> > 
> > server stream 0.0.0.0:0 upstream-proxy-tunnel
> > %[req.ssl_sni,lower,map_str(targets.map)]
> > ```
> > 
> > The targets.map should have something like this.
> > #dest proxy
> > sni01 proxy01
> > sni02 proxy02
> > 
> > I hope the background of upstream-proxy-target is now more clear.
> > 
> > > - In `server https_Via_Proxy1 0.0.0.0:0 upstream-proxy-tunnel 
> > > 127.0.0.1:3128`
> > >    from your config, what is 0.0.0.0:0 used for here? This binds to all 
> > > IPv4
> > >    but on a random free port?
> > 
> > This is required when the destination should be dynamic, it's documented 
> > here.
> > http://docs.haproxy.org/3.0/configuration.html#4.4-do-resolve
> > 
> > > A+
> > > Dave
> > 
> > Regards
> > Alex

Overall I find that there are plenty of good points in this patch and
the discussion around it. But it also raises questions:

  - IMHO the server's address should definitely be the proxy's address.
This will permit to use all the standard mechanisms we have such as
DNS resolution and health checks and continues to work as a regular
handshake. I'm not sure this is the case in the current state of the
patch since I'm seeing an extra "upstream-proxy-tunnel" parameter
(actually I'm confused by what you call a "target" here, as used in
upstream-proxy-target).

  - for the remote server's name, typically the one we'd extract from
a Host header usually or from SNI in some cases (maybe that's what
you called the "target" ?), we'd definitely need to support an
expression sent as text (usually ip:port), though we could as well
find it convenient to set the host and the port separately. In this
case it's likely that the sole presence of this expression could be
sufficient to enable the feature.

  - the Host field should default to the one in the URI if there's none
explicitly added.

  - I like the fact that you implemented support for headers upfront,
as we all know that this will be a prerequisite for many users.

  - you should definitely get rid of allthese DPRINTF() debug traces.
The simple fact that I no longer know how to enable them should ring
a bell about their relevance ;-)

  - we need to make sure this handshake is sent before SSL and not after.
This should permit to do what users normally expect, i.e., encrypt a
TCP connection and encapsulate it into a CONNECT request.

  - please don't realign the srv_kw_list at the end of the patch, it makes
it impossible to spot the addition(s) there. We can tolerate a few
unaligned entries (even though often it's a sign that a shorter name
would be welcome), and if you really need to realign, please do that
in a separate patch.

  - I think the headers addition should also be done into a separate patch
since it's a significant part of the patch. It will also enlighten the
default behavior since the code must work without extra header rules.

  - and BTW there's no need for this "upt" entry in the proxy struct, if
a string is needed you can just use an ist or even char* which are
simpler. But I do think that it should be extracted from the request
(something we need to figure how by default).

Thanks!
Willy

Re: Opinions desired on dropping support for duplicate names

[Aleksandar Lazic]

Hi.

On 2024-08-09 (Fr.) 17:24, Willy Tarreau wrote:

Hi all,

I'm continuing to find disgusting things in the code that are only here
for historical reasons which, in my opinion, should no longer exist.



[snipp]


Now we're at an era where many configs are generated in ways that cannot
even produce such corner cases, they're harder to follow and debug, and
I'm wondering what's the point of preserving support for such cases which
I'm intimately convinced that nobody relies on.

I'd be interested in opinions on some of these options:
   - deprecate duplicate server names for 3.1, requiring a global option
 to support them and drop their support for 3.3 ?


I would vote for this option.
+1


   - just drop the support of duplicate server names right now in 3.1 ?
   - keep these working "forever" and reconsider the option later, due to
 a very valid case that I'm ignoring ?
   - for backends + frontends, deprecate same names in 3.1, drop in 3.3,
 or drop now ? Or keep them ?

As you know I'm not in favor of removing stuff without prior warnings,
but these ones have become so absurd over time that sometimes I really
doubt anyone will ever run the code that deals with the error and the
code that parses the new option to force supporting the mechanism. We
could even consider backporting a warning for such cases in 3.0 so as
to limit the surprise if we were to drop that right now.


A warning for 3.0 is a good Idea, IMHO.


And I don't want to scare anyone, if there are valid cases, that's fine
as well and then I just want that we properly support them. It's just
that I feel that fixing the corner cases and costs above is pointless,
and without a use case I'm not tempted by investing time there.

If you're aware of other types of stupid duplicate identifiers that we
continue to support for historical reasons and that would make sense to
be simplified, please suggest!

Thanks in advance for sharing your opinion on this topic.
Willy


Regards
Alex


PS: let's also Cc Marko for the dataplane API since it should have impacts
 on it, though very likely it will simplify it, not make it more complex.

PPS: no emergency if you're on holiday, we can decide in a month as well.

PPPS: please no delirium about how we should take this opportunity to
   revist all the config language ;-)

Re: [RFC] Allow disabling abstract unix socket paths NUL-padding

[Willy Tarreau]

Hi Tristan,

I'm back on this topic (I had not forgotten it).

On Sat, Mar 09, 2024 at 07:02:34PM +, Tristan wrote:
> 
> 
> On 09/03/2024 18:09, Tristan wrote:
> > Hi Willy,
> > 
> > On 09/03/2024 16:51, Willy Tarreau wrote:
> > > Hi Tristan,
> > > 
> > > On Sat, Mar 09, 2024 at 04:20:21PM +, Tristan wrote:
> > > > To be honest, I don't think this is unfixable. It's just a matter of how
> > > > much code change we think is acceptable for it.
> > > 
> > > I don't mind about the amount of changes. "we've always done it like
> > > this"
> > > is never a valid excuse to keep code as it is, especially when it's wrong
> > > and causes difficulties to add new features. Of course I prefer invasive
> > > changes early in the dev cycle than late but we're still OK and I don't
> > > expect that many changes for this anyway.
> > 
> > Agreed on not shying away from amount of changes.
> > 
> > I tried to follow that route, being generous in what I was willing to
> > touch, but ran into the issue of protocols and socket families being a
> > 1:1 binding.
> > 
> > So adding a new socket family *requires* adding a new protocol too,
> > which isn't too hard but spirals a bit out of control:
> > - protocol_by_family is a misnomer, and it is actually protocols *by
> > socket domain* (which is still AF_UNIX, even with family AF_CUST_ABNS2,
> > and ends up overriding standard socket/abns protocol)
> > - but you can't just change that, because then other things stop working
> > as some places rely on socket domain != socket family
> 
> Actually, following up on this, I'm more and more convinced that it actually
> makes things somewhat worse to take this approach...
> 
> At least I don't see a good way to make it work without looking very bad,
> because in a few places we lookup a protocol based on a socket_storage
> reference, so we have truly only the socket domain to work with if no
> connection is established yet.
> 
> While it sounds like shying away from deeper changes, it seems to me that a
> bind/server option is truly the right way to go here.

I addressed these shortcomings of protocol lookups in a branch here:

  https://github.com/haproxy/haproxy/tree/20240809-abnsz-4

There was an API bug dating from 2.3 that was making everything more
complicated than necessary, the code was assigning the custom family
to sock_domain (the socket() argument) instead of sock_family (the
one we're supposed to store into the address itself). I fixed that
and added the minimal needed logic to properly perform the lookups
and also resolve custom families into their real ones, because
previously we had to perform random protocol lookups to figure them
(we didn't notice since other custom families don't map to real ones).
It even simplified a few places where some values were hard-coded in
certain calls, or some logic enforced late (e.g. log.c).

I now created abns as a custom proto, separate from uxst, so that it
has its own family. It was not strictly necessary but it will allow
us to get rid of plenty of \0 tests at many places by just having
functions dedicated to unix or abns. Even the addrcmp() function
should become simpler and I thought I'd specialize them.

I've just added a last patch to create abnsz (the zero-terminated
abns, since William rightfully suggested that the codename abns2
could make one think it's version 2), but for now it's a perfect
copy of the other one, I have not modified the address length checks.
I remember that you had a working PoC so I guess you've already spotted
all locations and what to change, so restarting from your patch set
will be much faster now.

Please let me know if you want to give it a try, or if you think you
won't have time to look into it and you prefer me to try to merge
your work into this, it's as you prefer.

I feel like we're finally seeing the light at the end of this long
tunnel!

I'm done for today and probably for the week-end as well I think.

Have a nice week-end!
Wily

Re: Opinions desired on dropping support for duplicate names

[Willy Tarreau]

Hi Tristan,

On Fri, Aug 09, 2024 at 03:51:24PM +, Tristan wrote:
> Hi Willy,
> 
> > On 9 Aug 2024, at 16:26, Willy Tarreau  wrote:
> > [...]
> > I'd be interested in opinions on some of these options:
> >  - deprecate duplicate server names for 3.1, requiring a global option
> >to support them and drop their support for 3.3 ?
> >  - just drop the support of duplicate server names right now in 3.1 ?
> >  - keep these working "forever" and reconsider the option later, due to
> >a very valid case that I'm ignoring ?
> 
> If I'd realized, I'd have assumed I found a bug I think; wonder how that even
> gets handled by things like the prom exporter which key purely by name?
> 
> +1 for drop in 3.1
> 
> >  - for backends + frontends, deprecate same names in 3.1, drop in 3.3,
> >or drop now ? Or keep them ?
> 
> For a pure proxy case without any load balancing (1 fe, 1 be, 1 server) I can
> see it being a bit heavy to have to name both, likely xyz-frontend and
> xyz-backend or something.  Is that justification enough? I'm not convinced.

That was the initial reason for that. But how many configs are using a
frontend and a backend with similar names ? Mystery. Even the stats are
confusing in such a case.

> Neutral.
> 
> wrt the stick tables example, the real crime is that naming them explicitly
> isn't mandatory, at both declaration and usage time... I'm not sure what the
> scope rules are for them, but I promptly decided it was not desirable
> knowledge when I started using them...

I contemplated the possibility of adding an optional "name" argument to
the "stick-table" directive a while ago. And I figured that it would make
it almost mandatory to totally decorellate the tables from the proxies.
These have already been split in the past since it's possible to declare
them in peers section, but that causes a configuration issue: if you merge
config fragments from various customers and they're allowed to name their
table as another customer's proxy, it becomes a bit annoying. It also
means having a same namespace for tables and backends, which reproduces
the issue we currently have with fronts and backs. Nothing unsolvable,
of course, but it definitely needs some thinking before doing a mistake.
BTW, for peers, the table's name contains the peers section name prepended
to it, e.g. "mypeers/t1". That might be something to do with fronts/backs
as well after all, which may also finally permit to support multiple tables
per proxy and avoid those scary configs with hundreds of empty backends
that just carry one stick-table each.

> > [...] We
> > could even consider backporting a warning for such cases in 3.0 so as
> > to limit the surprise if we were to drop that right now.
> 
> +1

Thanks for sharing your views on this!

Willy

Re: Opinions desired on dropping support for duplicate names

[Tristan]

Hi Willy,

> On 9 Aug 2024, at 16:26, Willy Tarreau  wrote:
> […]
> I'd be interested in opinions on some of these options:
>  - deprecate duplicate server names for 3.1, requiring a global option
>to support them and drop their support for 3.3 ?
>  - just drop the support of duplicate server names right now in 3.1 ?
>  - keep these working "forever" and reconsider the option later, due to
>a very valid case that I'm ignoring ?

If I’d realized, I’d have assumed I found a bug I think; wonder how that even 
gets handled by things like the prom exporter which key purely by name?

+1 for drop in 3.1

>  - for backends + frontends, deprecate same names in 3.1, drop in 3.3,
>or drop now ? Or keep them ?

For a pure proxy case without any load balancing (1 fe, 1 be, 1 server) I can 
see it being a bit heavy to have to name both, likely xyz-frontend and 
xyz-backend or something.  Is that justification enough? I’m not convinced.

Neutral.

wrt the stick tables example, the real crime is that naming them explicitly 
isn’t mandatory, at both declaration and usage time… I’m not sure what the 
scope rules are for them, but I promptly decided it was not desirable knowledge 
when I started using them…

> […] We
> could even consider backporting a warning for such cases in 3.0 so as
> to limit the surprise if we were to drop that right now.

+1

> Thanks in advance for sharing your opinion on this topic.
> Willy
> 
> PPPS: please no delirium about how we should take this opportunity to
>  revist all the config language ;-)

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

[Willy Tarreau]

Hi Matthieu,

On Fri, Aug 09, 2024 at 12:52:04PM +0200, Matthieu Baerts wrote:
> On 09/08/2024 11:32, Willy Tarreau wrote:
> > On Mon, May 06, 2024 at 02:10:02PM +0200, Björn Jacke wrote:
> >> 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.
> > 
> > I had a look at this series, and am seeing that the largest part of the
> > changes is to use ->sock_prot (either IPPROTO_TCP or IPPROTO_MPTCP) in
> > getsockopt() and setsockopt() calls, which is not in Dorian's patchset.
> > 
> > Thus I'm just wondering if this is mandatory or if there's a compatibility
> > mode in the kernel so that IPPROTO_TCP also works for MPTCP sockets. Maybe
> > Matthieu knows better and could enlighten us on this ?
> 
> There is no need to change anything in the getsockopt() and setsockopt()
> calls: it is still possible to use SOL_TCP (== IPPROTO_TCP). The kernel
> will try to adapt the socket option to the MPTCP case, e.g. applying
> something on all the subflows, or just the first one, or the MPTCP
> socket, depending on the option.
> 
> There are some socket options under SOL_MPTCP (284), but there are
> "new", and specific to MPTCP, e.g. to get info about the different paths
> being used, etc.
> 
> If you replace IPPROTO_TCP in the getsockopt() and setsockopt() calls by
> IPPROTO_MPTCP (262), it means you try to look at socket options with the
> SOL_X25 level, that's not what we want here ;)

OK, thank you for the very detailed explanation!

> > Because if it's required, we definitely need to apply similar changes
> > everywhere (the code has probably evolved a little bit since 2021), and
> > if not, it could mean that only the part that performs the protocol
> > registration (that Matthieu+Dorian also handled) and the doc should
> > suffice. I think we have everything in shape now to decide what's left
> > to do in order to get the feature merged. I hope this time we won't
> > miss the deadline!
> 
> In theory, the last patch I sent should be enough.
> 
> BTW, thank you for your other reply, no issues for the delay. I will try
> to look at the modifications you requested when I can find some time :)

That works for me. We'll release by end of november, with a hard freeze
around a month earlier, so there's no rush, though it always helps to
get feedback from users, of course!

If you'd figure that you can't find the time to complete it in a reasonable
delay, do not hesitate to ping me again about it to let me know, we'll find
how to arrange this together.

Also, I think it would be kind to leave a mention about Björn's work in
your commit message, who attempted the very first implementation 3-4
years ago when haproxy's internals were probably less ready to deal with
this, causing his work to be left pending for a while.

Thanks!
Willy

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

[Matthieu Baerts]

Hi Willy,

On 09/08/2024 11:32, Willy Tarreau wrote:
> Hi Björn,
> 
> I'm coming back to this:
> 
> On Mon, May 06, 2024 at 02:10:02PM +0200, Björn Jacke wrote:
>> 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.
> 
> I had a look at this series, and am seeing that the largest part of the
> changes is to use ->sock_prot (either IPPROTO_TCP or IPPROTO_MPTCP) in
> getsockopt() and setsockopt() calls, which is not in Dorian's patchset.
> 
> Thus I'm just wondering if this is mandatory or if there's a compatibility
> mode in the kernel so that IPPROTO_TCP also works for MPTCP sockets. Maybe
> Matthieu knows better and could enlighten us on this ?

There is no need to change anything in the getsockopt() and setsockopt()
calls: it is still possible to use SOL_TCP (== IPPROTO_TCP). The kernel
will try to adapt the socket option to the MPTCP case, e.g. applying
something on all the subflows, or just the first one, or the MPTCP
socket, depending on the option.

There are some socket options under SOL_MPTCP (284), but there are
"new", and specific to MPTCP, e.g. to get info about the different paths
being used, etc.

If you replace IPPROTO_TCP in the getsockopt() and setsockopt() calls by
IPPROTO_MPTCP (262), it means you try to look at socket options with the
SOL_X25 level, that's not what we want here ;)

> Because if it's required, we definitely need to apply similar changes
> everywhere (the code has probably evolved a little bit since 2021), and
> if not, it could mean that only the part that performs the protocol
> registration (that Matthieu+Dorian also handled) and the doc should
> suffice. I think we have everything in shape now to decide what's left
> to do in order to get the feature merged. I hope this time we won't
> miss the deadline!

In theory, the last patch I sent should be enough.

BTW, thank you for your other reply, no issues for the delay. I will try
to look at the modifications you requested when I can find some time :)

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

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

[Willy Tarreau]

Hi Björn,

I'm coming back to this:

On Mon, May 06, 2024 at 02:10:02PM +0200, Björn Jacke wrote:
> 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.

I had a look at this series, and am seeing that the largest part of the
changes is to use ->sock_prot (either IPPROTO_TCP or IPPROTO_MPTCP) in
getsockopt() and setsockopt() calls, which is not in Dorian's patchset.

Thus I'm just wondering if this is mandatory or if there's a compatibility
mode in the kernel so that IPPROTO_TCP also works for MPTCP sockets. Maybe
Matthieu knows better and could enlighten us on this ?

Because if it's required, we definitely need to apply similar changes
everywhere (the code has probably evolved a little bit since 2021), and
if not, it could mean that only the part that performs the protocol
registration (that Matthieu+Dorian also handled) and the doc should
suffice. I think we have everything in shape now to decide what's left
to do in order to get the feature merged. I hope this time we won't
miss the deadline!

Thanks,
Willy

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

[Willy Tarreau]

Hi Matthieu,

first, sorry for the long delay, but each time it's the same, the list
of pending urgent things drags me away. I'm back on this.

On Mon, Jun 03, 2024 at 05:33:31PM +0200, Matthieu Baerts wrote:
> >>> 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().
> > 
> > I *tend* to think that a new struct protocol is easier to add and
> > maintain. It could just share most (if not all) of the functions
> > with tcp, and probably be declared in the same file for ease of
> > sharing code.
> > 
> > I'm seeing that in the comment you linked above I also proposed a
> > keyword for "bind" and "server" lines, like we have "tfo" to enable
> > TCP fast-open. It tends to be slightly easier to implement but then
> > requires more care everywhere because such options do no apply to
> > all protocols, so if you have "mptcp on" on a "bind quic4@:443" line,
> > that's quite confusing so it deserves extra checks to make sure it
> > is not silently ignored. Also I do have some doubts about how to
> > retrieve the source address, maybe we'll find that in fact it should
> > be seen as a new address family and not a new transport layer. But I
> > think not at this point. And BTW Björn had apparently implemented a
> > solution based on mptcp@ as well so it's likely workable.
> 
> Yes, it looks better with a new 'struct protocol'.
> 
> I worked on a first draft a few weeks ago:
> 
>   https://github.com/matttbe/haproxy/commit/f48f36191
> 
> As I mentioned in a previous email, I can wait for you to be back to
> send a new version on the mailing list.

That looks pretty interesting and clean like this! I find your approach
of duplicating the tcp definition interesting, but I'd still prefer to
have it pre-defined rather than duplicated from tcp though. It's the only
one being done like this and I'm concerned that sooner or later we'll
break it the dirty way by checking that a list element is empty, that a
pointer is NULL or whatever. It's no big deal to have duplicated
definitions like this, and it eases "git grep" to find all affected
protocols when we want to touch a function.

I'm just wondering if the "alt" argument (that you renamed from ctrl_dgram)
is always exclusive with the mptcp one. It looks so at first glance. In
the worst case this would just end up with a bit field instead of just a
boolean, like many other functions.

But yes, overall I think that the approach looks like the correct one,
and it offers quite a lot of flexibility that way. Out of curiosity,
did you test it on the backend side ? I don't know what it implies to
do mptcp on the backend (if anything at all), it's more to make sure
we're not overlooking anything.

> >> Please note that Dorian is doing this as part of a work with his uni
> >> (useful contributions to Open-Source), and I think he would prefer
> >> sending the v2 before June, if that's OK. I can look at rebasing it
> >> later if needed. If there is no need to implement a fallback if the
> >> kernel doesn't support MPTCP, the patch should be smaller, it should be
> >> easy to rebase it.
> > 
> > Yeah, understood. I will have limited availability for the next two
> > weeks but if he needs some guidance to finish something almost done,
> > I'll do my best. It's important to encourage first-time contributors,
> > especially since it's easy to want to give up on a first patch feedback,
> > we've all known that :-)
> 
> Please take your time. As I mentioned in my previous email on the ML,
> Dorian is no longer available to work on that, so I will do my best to
> continue. No reason to rush, more to relax ;-)

Sure, but I'd like to get this merged before 3.1 is out ;-)

Feel free to send your patch(es) with a possibly updated commit messages
here on the list. I don't know if we can make a portable enough regtest
for this, especially if we consider that a silent fallback might lead to
TCP being used.

Thanks!
Willy

Re: [ANNOUNCE] haproxy-3.1-dev5

[Willy Tarreau]

Hi Ilya,

On Wed, Aug 07, 2024 at 08:30:46PM +0200,  ??? wrote:
> > HAProxy 3.1-dev5 was released on 2024/08/07. It added 88 new commits
> > after version 3.1-dev4.
> >
> > There were quite a bunch of fixes this time, spread over various areas
> > (h2, analysers, jwt, quic, 0-rtt, queues, traces), though nothing exciting
> > at this point.
> >
> 
> I'm not sure about roadmap of aws-lc support in HAProxy, but from my
> observation it reached parity with QuicTLS
> (*) performance of OpenSSL-1.1.1 (in some cases even 20% faster due to
> assembler implementation)
> (*) being actively developed by AWS (derived from BoringSSL)
> (*) all major QUIC features implemented
> (*) no LTS cycles ((

I totally agree. Its performance is even significantly higher than 1.1.1
(we've collected some measurements and are working on an article about
that). And indeed what's missing is LTS. I would really love it if the
QuicTLS and aws-lc teams could join efforts and try to maintain an LTS
version of this high-quality library! All the QUIC community would benefit
from this!

Willy

Re: [ANNOUNCE] haproxy-3.1-dev5

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

ср, 7 авг. 2024 г. в 18:48, Willy Tarreau :

> Hi,
>
> HAProxy 3.1-dev5 was released on 2024/08/07. It added 88 new commits
> after version 3.1-dev4.
>
> There were quite a bunch of fixes this time, spread over various areas
> (h2, analysers, jwt, quic, 0-rtt, queues, traces), though nothing exciting
> at this point.
>

I'm not sure about roadmap of aws-lc support in HAProxy, but from my
observation it reached parity with QuicTLS
(*) performance of OpenSSL-1.1.1 (in some cases even 20% faster due to
assembler implementation)
(*) being actively developed by AWS (derived from BoringSSL)
(*) all major QUIC features implemented
(*) no LTS cycles ((


>
> We've got a report of a user facing higher loads due to one of the new
> safety rules enforcements in the HTTP spec mandating that requests that
> contain both Transfer-Encoding and Content-Length had to work in close
> mode (to avoid smuggling on possible incompatible intermediary HTTP/1.0
> proxies), so we've added an option to relax this rule when the chain is
> trusted.
>
> On the QUIC side, Chacha20 and 0-RTT were fixed when using the aws-lc
> crypto library. A new ruleset "quic-initial" allows to filter packets
> during the QUIC handshake. The currently supported actions are "reject",
> "accept", "dgram-drop" (for a silent drop), and "send-retry" (to force
> a retry when in 0-RTT for example). It can significantly help against
> abuses or simply to enforce source-based filtering so that the client
> cannot even engage in a handshake. The quic traces will now also indicate
> how long a stream spent waiting for flow control, buffers, etc, which
> should help us explain why certain requests appear to be slow. Some
> Cubic-specific info are now also dumped in "show quic".
>
> The traces now permit some sources to follow other ones, so that when
> a source automatically triggers, the followers will automatically be
> enabled as well. One use case is to track the session, allowing to
> watch a communication between the frontend and the backend without
> being disturbed by the rest of the traffic. We've verified that we
> can follow a series of requests from a front QUIC connection to an
> HTTP/2 backend. This will allow us to simplify some captures. Also
> a new meta-source "all" is supported for some "trace" commands, to
> set the sink, the level and the source to follow. This will save a
> lot of debugging commands.
>
> A new pair of sample fetch functions, fs.debug_str() and bs.debug_str()
> can be used to complete the logs with useful debugging info from the
> lower layers (stream ID, flow-control etc). It appears important to
> continue to provide detailed troubleshooting elements because it has
> happened quite a few times since we have muxes that some logs would
> report an error, a timeout or something unusual and that it was a bit
> hard to figure what happened at the lower layers. Obviously with
> protocols like H2 and QUIC we can't tell the whole history but it should
> help quite a bit. For example the stream's pause times mentioned above
> will be there so it will be possible to correlate the request timers
> with some such elements.
>
> A more significant change concerns the loading of configuration files.
> Previously they were opened and parsed on the fly just once. With the
> pending master startup changes, the starting process will need to check
> in the global section if it's supposed to be the master process and stop
> there, and the worker will parse its own config. Since /dev/stdin works
> and is supported, it's not possible to open it twice. Instead it was
> decided that the config is pre-loaded in memory and processed from there
> so that it stays buffered. The config size in memory is not much of a
> concern given the huge amplification factor (40 to 100x) of a config
> represented in memory, so the temporary extra copy of the text-based one
> is small. There could be pitfalls, though and it's always interesting to
> know if you find something that breaks it. We already know that loading
> /dev/zero will make it eat a lot of RAM for example, but we'll rather
> address all corner cases as a whole than each of them individually.
>
> And the rest is as usual, some build fixes, CI updates and doc updates.
>
> Quite honestly, if you're running large configs and/or are streaming your
> configs over SSH to a remote daemon for example, or doing anything fancy,
> your feedback on the config loading changes is really important to those
> working on this. Normally you should not notice a difference at this step.
>
> 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.1/src/
>

Re: [PATCH] CI: keep logs for failed QIUC Interop jobs

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

patch attached.

ср, 7 авг. 2024 г. в 12:34, Илья Шипицин :

> please ignore, I'll send better patch
>
> ср, 7 авг. 2024 г. в 12:33, Ilia Shipitsin :
>
>> it might be useful to investigate logs of failed tests. to keep
>> artifacts small the following actions are taken
>> - only failed logs are kept
>> - logs retention is 6 days
>> ---
>>  .github/workflows/quic-interop-aws-lc.yml   | 13 +
>>  .github/workflows/quic-interop-libressl.yml | 13 +
>>  2 files changed, 26 insertions(+)
>>
>> diff --git a/.github/workflows/quic-interop-aws-lc.yml
>> b/.github/workflows/quic-interop-aws-lc.yml
>> index 1e0220d71..6f1bb38be 100644
>> --- a/.github/workflows/quic-interop-aws-lc.yml
>> +++ b/.github/workflows/quic-interop-aws-lc.yml
>> @@ -75,3 +75,16 @@ jobs:
>>pip install -r requirements.txt --break-system-packages
>>python run.py -l logs -r haproxy=ghcr.io/${{
>>  github.repository }}:aws-lc -t ${{
>> matrix.suite.tests }} -c ${{ matrix.suite.client }} -s haproxy
>>
>> +  - name: Delete succeeded logs
>> +if: failure()
>> +run: |
>> +  cd quic-interop-runner/logs/haproxy_${{ matrix.suite.client }}
>> +  cat ../../result.json | jq -r '.results[][] |
>> select(.result=="succeeded") | .name' | xargs rm -rf
>> +
>> +  - name: Logs upload
>> +if: failure()
>> +uses: actions/upload-artifact@v4
>> +with:
>> +  name: logs
>> +  path: quic-interop-runner/logs/
>> +  retention-days: 6
>> \ No newline at end of file
>> diff --git a/.github/workflows/quic-interop-libressl.yml
>> b/.github/workflows/quic-interop-libressl.yml
>> index 2a635bcfa..16850ec35 100644
>> --- a/.github/workflows/quic-interop-libressl.yml
>> +++ b/.github/workflows/quic-interop-libressl.yml
>> @@ -75,3 +75,16 @@ jobs:
>>pip install -r requirements.txt --break-system-packages
>>python run.py -l logs -r haproxy=ghcr.io/${{
>>  github.repository }}:libressl -t ${{
>> matrix.suite.tests }} -c ${{ matrix.suite.client }} -s haproxy
>>
>> +  - name: Delete succeeded logs
>> +if: failure()
>> +run: |
>> +  cd quic-interop-runner/logs/haproxy_${{ matrix.suite.client }}
>> +  cat ../../result.json | jq -r '.results[][] |
>> select(.result=="succeeded") | .name' | xargs rm -rf
>> +
>> +  - name: Logs upload
>> +if: failure()
>> +uses: actions/upload-artifact@v4
>> +with:
>> +  name: logs
>> +  path: quic-interop-runner/logs/
>> +  retention-days: 6
>> \ No newline at end of file
>> --
>> 2.43.0.windows.1
>>
>>
From 11e23ccb92d1612e47666d32b9ddd2321bb8e330 Mon Sep 17 00:00:00 2001
From: Ilia Shipitsin 
Date: Wed, 7 Aug 2024 13:02:29 +0200
Subject: [PATCH] CI: keep logs for failed QIUC Interop jobs

it might be useful to investigate logs of failed tests. to keep
artifacts small the following actions are taken
- only failed logs are kept
- logs retention is 6 days
---
 .github/workflows/quic-interop-aws-lc.yml   | 15 ++-
 .github/workflows/quic-interop-libressl.yml | 15 ++-
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/quic-interop-aws-lc.yml 
b/.github/workflows/quic-interop-aws-lc.yml
index 1e0220d71..f9144d9ed 100644
--- a/.github/workflows/quic-interop-aws-lc.yml
+++ b/.github/workflows/quic-interop-aws-lc.yml
@@ -73,5 +73,18 @@ jobs:
   git clone https://github.com/quic-interop/quic-interop-runner
   cd quic-interop-runner
   pip install -r requirements.txt --break-system-packages
-  python run.py -l logs -r haproxy=ghcr.io/${{ github.repository 
}}:aws-lc -t ${{ matrix.suite.tests }} -c ${{ matrix.suite.client }} -s haproxy
+  python run.py -j result.json -l logs -r haproxy=ghcr.io/${{ 
github.repository }}:aws-lc -t ${{ matrix.suite.tests }} -c ${{ 
matrix.suite.client }} -s haproxy
 
+  - name: Delete succeeded logs
+if: failure()
+run: |
+  cd quic-interop-runner/logs/haproxy_${{ matrix.suite.client }}
+  cat ../../result.json | jq -r '.results[][] | 
select(.result=="succeeded") | .name' | xargs rm -rf
+
+  - name: Logs upload
+if: failure()
+uses: actions/upload-artifact@v4
+with:
+  name: logs
+  path: quic-interop-runner/logs/
+  retention-days: 6
\ No newline at end of file
diff --git a/.github/workflows/quic-interop-libressl.yml 
b/.github/workflows/quic-interop-libressl.yml
index 2a635bcfa..d50a2a365 100644
--- a/.github/workflows/quic-interop-libressl.yml
+++ b/.github/workflows/quic-interop-libressl.yml
@@ -73,5 +73,18 @@ jobs:
   git clone https://github.com/quic-interop/quic-interop-runner
   cd quic-interop-runner
   pip install -r requirements.txt --break-system-packages
-  python run.py -l logs -r haproxy=ghcr.io/${{ github.repository 

Re: [PATCH] CI: keep logs for failed QIUC Interop jobs

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

please ignore, I'll send better patch

ср, 7 авг. 2024 г. в 12:33, Ilia Shipitsin :

> it might be useful to investigate logs of failed tests. to keep
> artifacts small the following actions are taken
> - only failed logs are kept
> - logs retention is 6 days
> ---
>  .github/workflows/quic-interop-aws-lc.yml   | 13 +
>  .github/workflows/quic-interop-libressl.yml | 13 +
>  2 files changed, 26 insertions(+)
>
> diff --git a/.github/workflows/quic-interop-aws-lc.yml
> b/.github/workflows/quic-interop-aws-lc.yml
> index 1e0220d71..6f1bb38be 100644
> --- a/.github/workflows/quic-interop-aws-lc.yml
> +++ b/.github/workflows/quic-interop-aws-lc.yml
> @@ -75,3 +75,16 @@ jobs:
>pip install -r requirements.txt --break-system-packages
>python run.py -l logs -r haproxy=ghcr.io/${{
>  github.repository }}:aws-lc -t ${{
> matrix.suite.tests }} -c ${{ matrix.suite.client }} -s haproxy
>
> +  - name: Delete succeeded logs
> +if: failure()
> +run: |
> +  cd quic-interop-runner/logs/haproxy_${{ matrix.suite.client }}
> +  cat ../../result.json | jq -r '.results[][] |
> select(.result=="succeeded") | .name' | xargs rm -rf
> +
> +  - name: Logs upload
> +if: failure()
> +uses: actions/upload-artifact@v4
> +with:
> +  name: logs
> +  path: quic-interop-runner/logs/
> +  retention-days: 6
> \ No newline at end of file
> diff --git a/.github/workflows/quic-interop-libressl.yml
> b/.github/workflows/quic-interop-libressl.yml
> index 2a635bcfa..16850ec35 100644
> --- a/.github/workflows/quic-interop-libressl.yml
> +++ b/.github/workflows/quic-interop-libressl.yml
> @@ -75,3 +75,16 @@ jobs:
>pip install -r requirements.txt --break-system-packages
>python run.py -l logs -r haproxy=ghcr.io/${{
>  github.repository }}:libressl -t ${{
> matrix.suite.tests }} -c ${{ matrix.suite.client }} -s haproxy
>
> +  - name: Delete succeeded logs
> +if: failure()
> +run: |
> +  cd quic-interop-runner/logs/haproxy_${{ matrix.suite.client }}
> +  cat ../../result.json | jq -r '.results[][] |
> select(.result=="succeeded") | .name' | xargs rm -rf
> +
> +  - name: Logs upload
> +if: failure()
> +uses: actions/upload-artifact@v4
> +with:
> +  name: logs
> +  path: quic-interop-runner/logs/
> +  retention-days: 6
> \ No newline at end of file
> --
> 2.43.0.windows.1
>
>

Re: [PR] Create SECURITY.md

[Willy Tarreau]

On Tue, Aug 06, 2024 at 09:59:41PM +0200, Nicolas CARPi wrote:
> Hello all,
> 
> I wrote a blog post regarding the availability of this file for the 
> biggest 5000 websites. Have a look if you're interested:
> 
> https://www.deltablot.com/posts/state-of-security-txt/

Thank you Nicolas!

Willy

Re: [PR] Create SECURITY.md

[Nicolas CARPi]

Hello all,

I wrote a blog post regarding the availability of this file for the 
biggest 5000 websites. Have a look if you're interested:

https://www.deltablot.com/posts/state-of-security-txt/

Best,
~Nicolas

Re: [PATCH] src/fcgi-app.c: handle strdup failure

[Willy Tarreau]

On Tue, Aug 06, 2024 at 08:35:21AM +0200,  ??? wrote:
> I'll provide better script. It is not actual patching, but detection (which
> looks like a patching)

Yes I know, I'm used to that as well. Our other scripts do that, they
provide a patch and that's used to detect in the end. But that's
convenient, particularly when combined with source control management
like git, as it suggests a change that's trivial to edit. The most
important is to locate places that need to be checked by a human, the
proposed fix is only a support for this and occasionally a time saver.

Willy

Re: [PATCH] src/fcgi-app.c: handle strdup failure

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

I'll provide better script. It is not actual patching, but detection (which
looks like a patching)

Thanks!

On Tue, Aug 6, 2024, 08:25 Willy Tarreau  wrote:

> On Tue, Aug 06, 2024 at 05:16:11AM +0200, Willy Tarreau wrote:
> > > diff --git a/src/fcgi-app.c b/src/fcgi-app.c
> > > index b3a9b7c59..98077b959 100644
> > > --- a/src/fcgi-app.c
> > > +++ b/src/fcgi-app.c
> > > @@ -606,6 +606,8 @@ static int proxy_parse_use_fcgi_app(char **args,
> int section, struct proxy *curp
> > > if (!fcgi_conf)
> > > goto err;
> > > fcgi_conf->name = strdup(args[1]);
> > > +   if (!fcgi_conf->name)
> > > +   goto err;
> > > LIST_INIT(&fcgi_conf->param_rules);
> > > LIST_INIT(&fcgi_conf->hdr_rules);
> >
> > Looks good to me, thanks! I think we can also commit your cocci script
> > as dev/coccinelle/strdup.cocci. We'll also mark the fix as BUG/MINOR.
>
> Now merged as two distinct commits, one for the script and one for the fix.
> Thank you Ilya!
> willy
>

Re: [PATCH] src/fcgi-app.c: handle strdup failure

[Willy Tarreau]

On Tue, Aug 06, 2024 at 05:16:11AM +0200, Willy Tarreau wrote:
> > diff --git a/src/fcgi-app.c b/src/fcgi-app.c
> > index b3a9b7c59..98077b959 100644
> > --- a/src/fcgi-app.c
> > +++ b/src/fcgi-app.c
> > @@ -606,6 +606,8 @@ static int proxy_parse_use_fcgi_app(char **args, int 
> > section, struct proxy *curp
> > if (!fcgi_conf)
> > goto err;
> > fcgi_conf->name = strdup(args[1]);
> > +   if (!fcgi_conf->name)
> > +   goto err;
> > LIST_INIT(&fcgi_conf->param_rules);
> > LIST_INIT(&fcgi_conf->hdr_rules);
> 
> Looks good to me, thanks! I think we can also commit your cocci script
> as dev/coccinelle/strdup.cocci. We'll also mark the fix as BUG/MINOR.

Now merged as two distinct commits, one for the script and one for the fix.
Thank you Ilya!
willy

Re: [PATCH] src/fcgi-app.c: handle strdup failure

[Willy Tarreau]

On Mon, Aug 05, 2024 at 09:02:46PM +0200,  ??? wrote:
> updated patch attached (I preferred that instead of sending with "git
> send-email")
> 
> ??, 5 ???. 2024 ?. ? 20:10,  ??? :
> 
> >
> >
> > ??, 5 ???. 2024 ?. ? 20:09, William Lallemand :
> >
> >> On Mon, Aug 05, 2024 at 08:01:39PM +0200,  ??? wrote:
> >> > Subject: Re: [PATCH] src/fcgi-app.c: handle strdup failure
> >> > ??, 5 ???. 2024 ?. ? 19:56, William Lallemand :
> >> >
> >> > > On Mon, Aug 05, 2024 at 07:17:48PM +0200, Ilia Shipitsin wrote:
> >> > > > Subject: [PATCH] src/fcgi-app.c: handle strdup failure
> >> > > > found by coccinelle
> >> > >
> >> > > Please add clearer commit messages in your patches, you tend to
> >> minimize
> >> > > them, thanks ! :-)
> >> > >
> >> >
> >> > truth to be told, I tend to write longer messages than usually :)
> >> >
> >> >
> >> > >
> >> > > > ---
> >> > > >  src/fcgi-app.c | 5 -
> >> > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> >> > > >
> >> > > > diff --git a/src/fcgi-app.c b/src/fcgi-app.c
> >> > > > index b3a9b7c59..d96bb222c 100644
> >> > > > --- a/src/fcgi-app.c
> >> > > > +++ b/src/fcgi-app.c
> >> > > > @@ -606,6 +606,8 @@ static int proxy_parse_use_fcgi_app(char
> >> **args, int
> >> > > section, struct proxy *curp
> >> > > >   if (!fcgi_conf)
> >> > > >   goto err;
> >> > > >   fcgi_conf->name = strdup(args[1]);
> >> > > > + if (!fcgi_conf->name)
> >> > > > + goto err;
> >> > > >   LIST_INIT(&fcgi_conf->param_rules);
> >> > > >   LIST_INIT(&fcgi_conf->hdr_rules);
> >> > > >
> >> > > > @@ -622,7 +624,8 @@ static int proxy_parse_use_fcgi_app(char
> >> **args, int
> >> > > section, struct proxy *curp
> >> > > >   return retval;
> >> > > >err:
> >> > > >   if (fcgi_conf) {
> >> > > > - free(fcgi_conf->name);
> >> > > > + if (fcgi_conf->name)
> >> > > > + free(fcgi_conf->name);
> >> > >
> >> > > You don't need to add a check there, free(NULL) does nothing.
> >> > >
> >> >
> >> > I tried to figure out whether that behaviour is by chance or by purpose
> >> > (taking into account variety of implementations on different OSes)
> >> >
> >> > I'll try again
> >> >
> >>
> >> This is a standard behavior that is specified in POSIX, honestly HAProxy
> >> won't probably run if it wasn't behaving this
> >> way on some weird OS.
> >>
> >> https://pubs.opengroup.org/onlinepubs/009695399/functions/free.html "If
> >> ptr is a null pointer, no action shall occur."
> >>
> >
> > "man 3 free" didn't give me any link to POSIX.
> > I've should have a look at POSIX directly, but I didn't
> >
> >
> >>
> >> --
> >> William Lallemand
> >>
> >

> From baa55f1434b2ed5288536f62b8e322ed20904b8d Mon Sep 17 00:00:00 2001
> From: Ilia Shipitsin 
> Date: Mon, 5 Aug 2024 20:59:09 +0200
> Subject: [PATCH] src/fcgi-app.c: handle strdup failure
> 
> this defect is found by the following coccinelle script:
> 
> // find calls to strdup
> @call@
> expression ptr;
> position p;
> @@
> 
> ptr@p = strdup(...);
> 
> // find ok calls to strdup
> @ok@
> expression ptr;
> position call.p;
> @@
> 
> ptr@p = strdup(...);
> ... when != ptr
> (
>  (ptr == NULL || ...)
> |
>  (ptr == 0 || ...)
> |
>  (ptr != NULL || ...)
> |
>  (ptr != 0 || ...)
> )
> 
> // fix bad calls to strdup
> @depends on !ok@
> expression ptr;
> position call.p;
> @@
> 
> ptr@p = strdup(...);
> + if (ptr == NULL) return;
> ---
>  src/fcgi-app.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/fcgi-app.c b/src/fcgi-app.c
> index b3a9b7c59..98077b959 100644
> --- a/src/fcgi-app.c
> +++ b/src/fcgi-app.c
> @@ -606,6 +606,8 @@ static int proxy_parse_use_fcgi_app(char **args, int 
> section, struct proxy *curp
>   if (!fcgi_conf)
>   goto err;
>   fcgi_conf->name = strdup(args[1]);
> + if (!fcgi_conf->name)
> + goto err;
>   LIST_INIT(&fcgi_conf->param_rules);
>   LIST_INIT(&fcgi_conf->hdr_rules);

Looks good to me, thanks! I think we can also commit your cocci script
as dev/coccinelle/strdup.cocci. We'll also mark the fix as BUG/MINOR.

Thanks!
Willy

Re: [PATCH] src/fcgi-app.c: handle strdup failure

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

updated patch attached (I preferred that instead of sending with "git
send-email")

пн, 5 авг. 2024 г. в 20:10, Илья Шипицин :

>
>
> пн, 5 авг. 2024 г. в 20:09, William Lallemand :
>
>> On Mon, Aug 05, 2024 at 08:01:39PM +0200, Илья Шипицин wrote:
>> > Subject: Re: [PATCH] src/fcgi-app.c: handle strdup failure
>> > пн, 5 авг. 2024 г. в 19:56, William Lallemand :
>> >
>> > > On Mon, Aug 05, 2024 at 07:17:48PM +0200, Ilia Shipitsin wrote:
>> > > > Subject: [PATCH] src/fcgi-app.c: handle strdup failure
>> > > > found by coccinelle
>> > >
>> > > Please add clearer commit messages in your patches, you tend to
>> minimize
>> > > them, thanks ! :-)
>> > >
>> >
>> > truth to be told, I tend to write longer messages than usually :)
>> >
>> >
>> > >
>> > > > ---
>> > > >  src/fcgi-app.c | 5 -
>> > > >  1 file changed, 4 insertions(+), 1 deletion(-)
>> > > >
>> > > > diff --git a/src/fcgi-app.c b/src/fcgi-app.c
>> > > > index b3a9b7c59..d96bb222c 100644
>> > > > --- a/src/fcgi-app.c
>> > > > +++ b/src/fcgi-app.c
>> > > > @@ -606,6 +606,8 @@ static int proxy_parse_use_fcgi_app(char
>> **args, int
>> > > section, struct proxy *curp
>> > > >   if (!fcgi_conf)
>> > > >   goto err;
>> > > >   fcgi_conf->name = strdup(args[1]);
>> > > > + if (!fcgi_conf->name)
>> > > > + goto err;
>> > > >   LIST_INIT(&fcgi_conf->param_rules);
>> > > >   LIST_INIT(&fcgi_conf->hdr_rules);
>> > > >
>> > > > @@ -622,7 +624,8 @@ static int proxy_parse_use_fcgi_app(char
>> **args, int
>> > > section, struct proxy *curp
>> > > >   return retval;
>> > > >err:
>> > > >   if (fcgi_conf) {
>> > > > - free(fcgi_conf->name);
>> > > > + if (fcgi_conf->name)
>> > > > + free(fcgi_conf->name);
>> > >
>> > > You don't need to add a check there, free(NULL) does nothing.
>> > >
>> >
>> > I tried to figure out whether that behaviour is by chance or by purpose
>> > (taking into account variety of implementations on different OSes)
>> >
>> > I'll try again
>> >
>>
>> This is a standard behavior that is specified in POSIX, honestly HAProxy
>> won't probably run if it wasn't behaving this
>> way on some weird OS.
>>
>> https://pubs.opengroup.org/onlinepubs/009695399/functions/free.html "If
>> ptr is a null pointer, no action shall occur."
>>
>
> "man 3 free" didn't give me any link to POSIX.
> I've should have a look at POSIX directly, but I didn't
>
>
>>
>> --
>> William Lallemand
>>
>
From baa55f1434b2ed5288536f62b8e322ed20904b8d Mon Sep 17 00:00:00 2001
From: Ilia Shipitsin 
Date: Mon, 5 Aug 2024 20:59:09 +0200
Subject: [PATCH] src/fcgi-app.c: handle strdup failure

this defect is found by the following coccinelle script:

// find calls to strdup
@call@
expression ptr;
position p;
@@

ptr@p = strdup(...);

// find ok calls to strdup
@ok@
expression ptr;
position call.p;
@@

ptr@p = strdup(...);
... when != ptr
(
 (ptr == NULL || ...)
|
 (ptr == 0 || ...)
|
 (ptr != NULL || ...)
|
 (ptr != 0 || ...)
)

// fix bad calls to strdup
@depends on !ok@
expression ptr;
position call.p;
@@

ptr@p = strdup(...);
+ if (ptr == NULL) return;
---
 src/fcgi-app.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/fcgi-app.c b/src/fcgi-app.c
index b3a9b7c59..98077b959 100644
--- a/src/fcgi-app.c
+++ b/src/fcgi-app.c
@@ -606,6 +606,8 @@ static int proxy_parse_use_fcgi_app(char **args, int 
section, struct proxy *curp
if (!fcgi_conf)
goto err;
fcgi_conf->name = strdup(args[1]);
+   if (!fcgi_conf->name)
+   goto err;
LIST_INIT(&fcgi_conf->param_rules);
LIST_INIT(&fcgi_conf->hdr_rules);
 
-- 
2.43.0.windows.1

Re: [PATCH] src/fcgi-app.c: handle strdup failure

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

пн, 5 авг. 2024 г. в 20:09, William Lallemand :

> On Mon, Aug 05, 2024 at 08:01:39PM +0200, Илья Шипицин wrote:
> > Subject: Re: [PATCH] src/fcgi-app.c: handle strdup failure
> > пн, 5 авг. 2024 г. в 19:56, William Lallemand :
> >
> > > On Mon, Aug 05, 2024 at 07:17:48PM +0200, Ilia Shipitsin wrote:
> > > > Subject: [PATCH] src/fcgi-app.c: handle strdup failure
> > > > found by coccinelle
> > >
> > > Please add clearer commit messages in your patches, you tend to
> minimize
> > > them, thanks ! :-)
> > >
> >
> > truth to be told, I tend to write longer messages than usually :)
> >
> >
> > >
> > > > ---
> > > >  src/fcgi-app.c | 5 -
> > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/src/fcgi-app.c b/src/fcgi-app.c
> > > > index b3a9b7c59..d96bb222c 100644
> > > > --- a/src/fcgi-app.c
> > > > +++ b/src/fcgi-app.c
> > > > @@ -606,6 +606,8 @@ static int proxy_parse_use_fcgi_app(char **args,
> int
> > > section, struct proxy *curp
> > > >   if (!fcgi_conf)
> > > >   goto err;
> > > >   fcgi_conf->name = strdup(args[1]);
> > > > + if (!fcgi_conf->name)
> > > > + goto err;
> > > >   LIST_INIT(&fcgi_conf->param_rules);
> > > >   LIST_INIT(&fcgi_conf->hdr_rules);
> > > >
> > > > @@ -622,7 +624,8 @@ static int proxy_parse_use_fcgi_app(char **args,
> int
> > > section, struct proxy *curp
> > > >   return retval;
> > > >err:
> > > >   if (fcgi_conf) {
> > > > - free(fcgi_conf->name);
> > > > + if (fcgi_conf->name)
> > > > + free(fcgi_conf->name);
> > >
> > > You don't need to add a check there, free(NULL) does nothing.
> > >
> >
> > I tried to figure out whether that behaviour is by chance or by purpose
> > (taking into account variety of implementations on different OSes)
> >
> > I'll try again
> >
>
> This is a standard behavior that is specified in POSIX, honestly HAProxy
> won't probably run if it wasn't behaving this
> way on some weird OS.
>
> https://pubs.opengroup.org/onlinepubs/009695399/functions/free.html "If
> ptr is a null pointer, no action shall occur."
>

"man 3 free" didn't give me any link to POSIX.
I've should have a look at POSIX directly, but I didn't


>
> --
> William Lallemand
>

Re: [PATCH] src/fcgi-app.c: handle strdup failure

[William Lallemand]

On Mon, Aug 05, 2024 at 08:01:39PM +0200, Илья Шипицин wrote:
> Subject: Re: [PATCH] src/fcgi-app.c: handle strdup failure
> пн, 5 авг. 2024 г. в 19:56, William Lallemand :
> 
> > On Mon, Aug 05, 2024 at 07:17:48PM +0200, Ilia Shipitsin wrote:
> > > Subject: [PATCH] src/fcgi-app.c: handle strdup failure
> > > found by coccinelle
> >
> > Please add clearer commit messages in your patches, you tend to minimize
> > them, thanks ! :-)
> >
> 
> truth to be told, I tend to write longer messages than usually :)
> 
> 
> >
> > > ---
> > >  src/fcgi-app.c | 5 -
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/fcgi-app.c b/src/fcgi-app.c
> > > index b3a9b7c59..d96bb222c 100644
> > > --- a/src/fcgi-app.c
> > > +++ b/src/fcgi-app.c
> > > @@ -606,6 +606,8 @@ static int proxy_parse_use_fcgi_app(char **args, int
> > section, struct proxy *curp
> > >   if (!fcgi_conf)
> > >   goto err;
> > >   fcgi_conf->name = strdup(args[1]);
> > > + if (!fcgi_conf->name)
> > > + goto err;
> > >   LIST_INIT(&fcgi_conf->param_rules);
> > >   LIST_INIT(&fcgi_conf->hdr_rules);
> > >
> > > @@ -622,7 +624,8 @@ static int proxy_parse_use_fcgi_app(char **args, int
> > section, struct proxy *curp
> > >   return retval;
> > >err:
> > >   if (fcgi_conf) {
> > > - free(fcgi_conf->name);
> > > + if (fcgi_conf->name)
> > > + free(fcgi_conf->name);
> >
> > You don't need to add a check there, free(NULL) does nothing.
> >
> 
> I tried to figure out whether that behaviour is by chance or by purpose
> (taking into account variety of implementations on different OSes)
> 
> I'll try again
> 

This is a standard behavior that is specified in POSIX, honestly HAProxy won't 
probably run if it wasn't behaving this
way on some weird OS.

https://pubs.opengroup.org/onlinepubs/009695399/functions/free.html "If ptr is 
a null pointer, no action shall occur."

-- 
William Lallemand

Re: [PATCH] src/fcgi-app.c: handle strdup failure

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

пн, 5 авг. 2024 г. в 19:56, William Lallemand :

> On Mon, Aug 05, 2024 at 07:17:48PM +0200, Ilia Shipitsin wrote:
> > Subject: [PATCH] src/fcgi-app.c: handle strdup failure
> > found by coccinelle
>
> Please add clearer commit messages in your patches, you tend to minimize
> them, thanks ! :-)
>

truth to be told, I tend to write longer messages than usually :)


>
> > ---
> >  src/fcgi-app.c | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/fcgi-app.c b/src/fcgi-app.c
> > index b3a9b7c59..d96bb222c 100644
> > --- a/src/fcgi-app.c
> > +++ b/src/fcgi-app.c
> > @@ -606,6 +606,8 @@ static int proxy_parse_use_fcgi_app(char **args, int
> section, struct proxy *curp
> >   if (!fcgi_conf)
> >   goto err;
> >   fcgi_conf->name = strdup(args[1]);
> > + if (!fcgi_conf->name)
> > + goto err;
> >   LIST_INIT(&fcgi_conf->param_rules);
> >   LIST_INIT(&fcgi_conf->hdr_rules);
> >
> > @@ -622,7 +624,8 @@ static int proxy_parse_use_fcgi_app(char **args, int
> section, struct proxy *curp
> >   return retval;
> >err:
> >   if (fcgi_conf) {
> > - free(fcgi_conf->name);
> > + if (fcgi_conf->name)
> > + free(fcgi_conf->name);
>
> You don't need to add a check there, free(NULL) does nothing.
>

I tried to figure out whether that behaviour is by chance or by purpose
(taking into account variety of implementations on different OSes)

I'll try again

>
> >   free(fcgi_conf);
> >   }
> >   memprintf(err, "out of memory");
> > --
> > 2.43.0.windows.1
> >
>
> --
> William Lallemand
>

Re: [PATCH] src/fcgi-app.c: handle strdup failure

[William Lallemand]

On Mon, Aug 05, 2024 at 07:17:48PM +0200, Ilia Shipitsin wrote:
> Subject: [PATCH] src/fcgi-app.c: handle strdup failure
> found by coccinelle

Please add clearer commit messages in your patches, you tend to minimize them, 
thanks ! :-)

> ---
>  src/fcgi-app.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/fcgi-app.c b/src/fcgi-app.c
> index b3a9b7c59..d96bb222c 100644
> --- a/src/fcgi-app.c
> +++ b/src/fcgi-app.c
> @@ -606,6 +606,8 @@ static int proxy_parse_use_fcgi_app(char **args, int 
> section, struct proxy *curp
>   if (!fcgi_conf)
>   goto err;
>   fcgi_conf->name = strdup(args[1]);
> + if (!fcgi_conf->name)
> + goto err;
>   LIST_INIT(&fcgi_conf->param_rules);
>   LIST_INIT(&fcgi_conf->hdr_rules);
>  
> @@ -622,7 +624,8 @@ static int proxy_parse_use_fcgi_app(char **args, int 
> section, struct proxy *curp
>   return retval;
>err:
>   if (fcgi_conf) {
> - free(fcgi_conf->name);
> + if (fcgi_conf->name)
> + free(fcgi_conf->name);

You don't need to add a check there, free(NULL) does nothing.

>   free(fcgi_conf);
>   }
>   memprintf(err, "out of memory");
> -- 
> 2.43.0.windows.1
> 

-- 
William Lallemand

Re: [PR] Create SECURITY.md

[Willy Tarreau]

On Sat, Aug 03, 2024 at 12:23:03PM +, PR Bot wrote:
> Dear list!
> 
> Author: Valen1393 
> Number of patches: 1
> 
> This is an automated relay of the Github pull request:
>Create SECURITY.md
> 
> Patch title(s): 
>Create SECURITY.md
> 
> Link:
>https://github.com/haproxy/haproxy/pull/2661
> 
> Edit locally:
>wget https://github.com/haproxy/haproxy/pull/2661.patch && vi 2661.patch
> 
> Apply locally:
>curl https://github.com/haproxy/haproxy/pull/2661.patch | git am -
> 
> Description:
>Sabar

Not very descriptive commit message... Anyway, I don't see any value in
this. First, the address to contact for security reports is already
present in the doc (and some find it since we occasionally receive
messages there). Second, the list of supported versions is already on
the web site, and the maintenance status of each version as well as the
planned EOL is even reported in the executables themselves, so at best
this new file would add nothing, at worst it would add extra maintenance
and possible confusion when out of sync.

Thanks,
Willy