Re: [PATCH 1/1] CI: VTest: accelerate package install a bit

2024-05-30 Thread Willy Tarreau
On Thu, May 30, 2024 at 04:12:02PM +0200, William Lallemand wrote:
> On Thu, May 30, 2024 at 03:40:31PM +0200, Ilia Shipitsin wrote:
> > Subject: [PATCH 1/1] CI: VTest: accelerate package install a bit
> > let's check and install only package is required
> > ---
> >  .github/workflows/vtest.yml | 8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/.github/workflows/vtest.yml b/.github/workflows/vtest.yml
> > index f862dc5a7..5208df757 100644
> > --- a/.github/workflows/vtest.yml
> > +++ b/.github/workflows/vtest.yml
> > @@ -82,10 +82,10 @@ jobs:
> >run: |
> >  sudo apt-get update
> >  sudo apt-get --no-install-recommends -y install \
> > -  liblua5.4-dev \
> > -  libpcre2-dev \
> > -  libsystemd-dev \
> > -  ninja-build \
> > +  ${{ contains(matrix.FLAGS, 'USE_LUA=1') && 'liblua5.4-dev'  
> > || '' }} \
> > +  ${{ contains(matrix.FLAGS, 'USE_PCRE2=1')   && 'libpcre2-dev'   
> > || '' }} \
> > +  ${{ contains(matrix.FLAGS, 'USE_SYSTEMD')   && 'libsystemd-dev' 
> > || '' }} \
> 
> In fact libsystemd-dev is not required anymore, we have our own sd_notify in 
> haproxy.

I thought we still needed it for the .h but apparently not, indeed.

Willy



Re: [PATCH 1/1] CI: VTest: accelerate package install a bit

2024-05-30 Thread Willy Tarreau
Hi Ilya,

On Thu, May 30, 2024 at 03:40:31PM +0200, Ilia Shipitsin wrote:
> +  ${{ contains(matrix.FLAGS, 'USE_LUA=1') && 'liblua5.4-dev'  || 
> '' }} \
> +  ${{ contains(matrix.FLAGS, 'USE_PCRE2=1')   && 'libpcre2-dev'   || 
> '' }} \
> +  ${{ contains(matrix.FLAGS, 'USE_SYSTEMD')   && 'libsystemd-dev' || 
> '' }} \
   ^^^ =1 ?
while I guess it should work, is this an overlook or is it intentional
that "=1" is not specified here ?

Thanks,
Willy



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

2024-05-30 Thread Willy Tarreau
Hi Matthieu,

finally a bit more available again...

On Fri, Apr 26, 2024 at 06:34:02PM +0200, Matthieu Baerts wrote:
> > I *am* interested in the feature, which has been
> > floating around for a few years already. However I tend to agree with
> > Nicolas that, at least for the principle of least surprise, I'd rather
> > not have this turned on by default. There might even be security
> > implications that some are not willing to take yet, and forcing them
> > to guess that they need to opt out of something is not great in general
> > (it might change in the future though, once everyone turns it on by
> > default, like we did for keep-alive, threads, and h2 for example).
> 
> It makes sense as well! I initially suggested to Dorian to first send a
> patch where MPTCP is enabled by default because of the low impact (and
> also after having seen an old comment on that topic [1]). But indeed,
> doing that in steps sounds safer.
> 
> [1] https://github.com/haproxy/haproxy/issues/1028#issuecomment-754560146

Yeah I understand, it would essentially depend on how all of this evolves
over time and how adoption grows for mptcp. Maybe one day most users will
expect it to work by default.

> > I'm also concerned by the change at the socket layer that will make all
> > new sockets cause two syscalls on systems where this is not enabled,
> 
> I thought the listening sockets were created only once at startup and
> having two syscalls would have been fine. I guess it should be easy to
> remember the failure the first time, to avoid the extra syscalls for the
> next listening sockets.

What you say is true for the listeners, but the socket() call is also
used when connecting to a backend server (though maybe I missed something
during the review).

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

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

I don't see how it should be related. The rhttp part is always tricky,
but the concept is that we accept a regular connection on a regular
frontend, then based on an explicit rule, return it and assign it as
an idle conn to a server. And for the server setting up pre-connected
idle conns to the gateway, it's just a regular address as well and the
protocol should not be involved there.

> > Regarding how
> > we'd deal with the fallback, we'll see, but overall, thinking that
> > would explicitly configure mptcp and not even get an error if
> > it cannot bind is problematic to me, because among the most common
> > causes of trouble are things that everyone ignores (module not loaded,
> > missing secondary IP, firewall rules etc) that usually cause problems.
> > Those we can discover at boot time should definitely be reported.
> 
> With the v1 Dorian suggested where MPTCP is enabled by default, a
> warning is reported if it is not possible to create an MPTCP socket, and
> a "plain" TCP one has been created instead.
> 
> If MPTCP is explicitly asked, I guess it would make sense to fail if it
> is not possible to create an MPTCP socket, no?

Yes I agree. I anticipate that if the solution eventually becomes
successful, some users will then want a 3-state setting: always-on,
always-off, best-effort. But let's not needlessly complexify the design
for now.

> > But
> > let's discuss this in early June instead (I mean, feel free to discuss
> > the points together, but I'm not going to invest more time on this at
> > this moment).
> 
> Thank you!
> 
> 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 

Re: [PATCH v2] FEATURE: add opt-in MPTCP support

2024-05-30 Thread Willy Tarreau
Hi Dorian,

I'm now done with the release and having more time to read your
work. First, thanks for this update. I understand that you're almost
running out of time on this topic which must be completed before
June so I'm not going to make you waste your time. Some comments
below.

On Thu, May 16, 2024 at 03:53:40PM +0200, Dorian Craps wrote:
> From: Dorian Craps 
> 
> 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 the "mptcp" global option in the config, which
> allows the creation of an MPTCP socket instead of TCP on Linux. If
> Multipath TCP is not supported on the system, an error will be reported,
> and the application will stop.
> 
> A test has been added, it is a copy of "default_rules.vtc" in tcp-rules
> with the addition of "mptcp" in the config. I'm not sure what else needs
> to be tested for the moment, with this global MPTCP option.
> 
> Note: another patch is coming to support enabling MPTCP per address, but
> I prefer to already send this patch, just in case, as I will soon have
> less time to dedicate to this.

Thanks for this. As I previously mentioned, I'm not going to merge a
patch with a global option that does all or nothing, however having a
working patch like this can definitely serve as a proof-of-concept and
a reference when testing a more granular approach, and in this regard
your patch is much welcome!

> Due to the limited impact within a data center environment,
> we have decided not to implement MPTCP between the proxy and the servers.
> The high-speed, low-latency nature of data center networks reduces
> the benefits of MPTCP, making the complexity of its implementation
> unnecessary in this context.

I definitely understand. However let's keep in mind that whenever we
envision a use case, someone will come with a different one and suggest
that we should support it. Here I can imagine that some CDN operators
might be interested in using MPTCP to the origin server as well by
connecting via multiple paths. This opens a big can of worms with
questions about source address binding, interface binding etc, which
cannot be addressed easily as they'll all have impacts on the way
servers are configured. So I'm totally fine with having only a frontend
support in a first time. I just mentioned this to give you more context
and so that you know it's not only the DC that's on the backend, even
if it's by far the most common, and what some of the complex aspects
can be.

I'll be in vacation for two weeks at the end of this week, will you need
some review of a possible next patch, or did you give up due to some
difficulties you faced while exploring the per-listener configuration ?

I'll give you a few comments on the patch below:

> diff --git a/src/cfgparse-global.c b/src/cfgparse-global.c
> index b173511c9..0feccd4b2 100644
> --- a/src/cfgparse-global.c
> +++ b/src/cfgparse-global.c
> @@ -23,6 +23,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  
>  int cluster_secret_isset;
> @@ -52,7 +54,7 @@ static const char *common_kw_list[] = {
>   "presetenv", "unsetenv", "resetenv", "strict-limits", "localpeer",
>   "numa-cpu-mapping", "defaults", "listen", "frontend", "backend",
>   "peers", "resolvers", "cluster-secret", "no-quic", "limited-quic",
> - NULL /* must be last */
> + "mptcp", NULL /* must be last */
>  };

Style only: better leave the NULL alone on its line so that it remains
distinct from the rest.

>  /*
> @@ -1334,6 +1336,23 @@ int cfg_parse_global(const char *file, int linenum, 
> char **args, int kwm)
>   HA_ATOMIC_STORE(_key, tmp);
>   }
>   }
> + else if (strcmp(args[0], "mptcp") == 0) {
> + if (alertif_too_many_args(0, file, linenum, args, _code))
> + goto out;
> +#ifdef __linux__
> +#ifndef IPPROTO_MPTCP
> +#define IPPROTO_MPTCP 262
> +#endif

This would be better placed in include/haproxy/compat.h (there are
already other ones there).

> +  

Re: [PATCH 0/3] CI: preparation for Ubuntu 24.04

2024-05-29 Thread Willy Tarreau
On Wed, May 29, 2024 at 09:59:13PM +0200, Ilia Shipitsin wrote:
> GitHub has launched Ubuntu 24.04 runners in beta. 
> While runners are not yet stable, switching to them
> has shown some inconsistance in pipeline which is better
> to be resolved before actual upgrade to Ubuntu 24.04
> 
> Ilia Shipitsin (3):
>   CI: use "--no-install-recommends" for apt-get
>   CI: switch to lua 5.4
>   CI: use USE_PCRE2 instead of USE_PCRE

Whole series applied, thank you Ilya!
Willy



Re: [PATCH 1/2] REGTESTS: Remove REQUIRE_VERSION=2.1 from all tests

2024-05-29 Thread Willy Tarreau
On Wed, May 29, 2024 at 07:55:32PM +0200, Tim Duesterhus wrote:
> HAProxy 2.2 is the lowest supported version, thus this always matches.

(...)

Both patches applied, thank you Tim!
Willy



Re: [ANNOUNCE] haproxy-3.0.0

2024-05-29 Thread Willy Tarreau
Hi Tim,

On Wed, May 29, 2024 at 07:48:10PM +0200, Tim Düsterhus wrote:
> Hi
> 
> On 5/29/24 17:07, Willy Tarreau wrote:
> > HAProxy 3.0.0 was released on 2024/05/29.
> 
> Congratulations on the successful release!

Thanks!

> I've just opened a PR for the "Official Docker Images" to add HAProxy 3.1:
> https://github.com/docker-library/haproxy/pull/234

OK!

> And of course it wouldn't be a real release, without me pointing out some
> things here and there :-)

It's because you stress me :-)

> - I've made a little change for 2.9 on the landing page of the docs:
> https://github.com/haproxy/docs/commit/94ec86972c6676b132954c0b4d629ca4287ae449

Yes, I saw that, thank you!

> - The version table on haproxy.org still has the EOL column for 2.0 in bold.
> Other EOL versions are not bold, so that's inconsistent.

Ah, that makes sense, you're right. Now fixed!

> Enjoy your vacation!

I'll try ;-)

Willy



[ANNOUNCE] haproxy-3.0.0

2024-05-29 Thread Willy Tarreau
went through this tedious task of enumerating
the changes, and it will be posted soon here:

  https://www.haproxy.com/blog/announcing-haproxy-3-0

My understanding is that there will be some followups with a focus on
selected points. I'm not surprised by the difficulty of the exercise
this time ;-)

For this version, we've got an increased help from various testers who
accepted to run one (or a few) servers with the development version, and
who were able to report a few problems with accurate version ranges, as
well as traces and info that permitted to fix the issues quickly. It
worked amazingly well and allowed us to address some nasty bugs that are
fairly hard to reproduce and that were present for several versions
already. At the risk of repeating myself, thanks for that! I know that
operating a -dev version requires a bit more involvement than a stable one
but it's also a win-win: when something doesn't please you, it's not too
late to suggest a change, and you can benefit from the latest debugging
features and performance improvements. I sincerely hope that this success
will encourage other users into that direction. The nice benefit for the
user of facing a bug in -dev vs -stable is that we have no problem
developing new debugging extensions just for that issue, so a git pull is
enough to suddenly make the problem much more observable and require less
amount of work to filter data than with a stable version. And something
that's human is that developers tend to be much more attracted by issues
affecting areas that are still fresh in their heads and will tend to treat
them with higher priority.

I also noticed more exchanges from various participants on the issues
and here on the list, so big thanks as well to those who take time to
review other users' problem reports and requests for help. Especially
for first-time reporters, it gives them a great experience of the
project and its community.

As usual with a new major release comes the death of an old one. This time
it's 2.0 that passed away after 5 years serving as a transition between
the old legacy versions and the newer HTX-enabled ones. I'm fairly sure
there are still some here and there, so please consider this as a reminder
that it's about time to upgrade. And 2.4 turned to critical fixes only
status.

On a side note (not very funny but surprising), apparently there was a big
GitHub outage last night, and this morning we're getting a "Ooops 500" page
on the haproxy repository there: https://github.com/haproxy/haproxy
The issues seem to be working, the wiki and docs projects as well. So I
suspect that an error page got cached during the outage and continues to
be delivered for whetever reason. I opened a ticket to their support and
we'll see when we get a response. Fortunately we're not completely blocked,
but it feels strange to release on a day of outage. After all, that's a
form of resilience that also makes one use a load balancer, so there's
some logic there.

Speaking of resilience, I'm going to take a bit of vacation next week and
the week after (maybe I should have postponed given the heavy rain here),
but you're in good hands with the rest of the team, and Christopher is
back on Monday, fresh an in full force. Maybe you'll even manage to
convince him to emit -dev1 himself, who knows :-)

Please find the usual URLs below :
   Site index   : https://www.haproxy.org/
   Documentation: https://docs.haproxy.org/
   Wiki : https://github.com/haproxy/wiki/wiki
   Discourse: https://discourse.haproxy.org/
   Slack channel: https://slack.haproxy.org/
   Issue tracker: https://github.com/haproxy/haproxy/issues
   Sources  : https://www.haproxy.org/download/3.0/src/
   Git repository   : https://git.haproxy.org/git/haproxy-3.0.git/
   Git Web browsing : https://git.haproxy.org/?p=haproxy-3.0.git
   Changelog: https://www.haproxy.org/download/3.0/src/CHANGELOG
   Dataplane API: 
https://github.com/haproxytech/dataplaneapi/releases/latest
   Pending bugs : https://www.haproxy.org/l/pending-bugs
   Reviewed bugs: https://www.haproxy.org/l/reviewed-bugs
   Code reports : https://www.haproxy.org/l/code-reports
   Latest builds: https://www.haproxy.org/l/dev-packages

I verified what I had in mind for 3.0 and 3.1-dev0 (that just opened),
and I think all is good (Tim already fixed an incorrect color on the
docs index). As usual, if (should I say when?) you detect a broken link,
just let me know so I can fix it.

Have fun!
Willy
---
Complete changelog from 3.0-dev13:
Amaury Denoyelle (2):
  DOC: streamline http-reuse and connection naming definition
  REGTESTS: complete http-reuse test with pool-conn-name

Aurelien DARRAGON (3):
  MINOR: log: rename 'log-format tag' to 'log-format alias'
  DOC: config: document logformat item naming and typecasting features
  DOC: config: add %ID logformat alias alternative

Valentine Krasnobaeva (3):
  CLEANUP: ssl/ocsp

[ANNOUNCE] haproxy-3.0-dev13

2024-05-24 Thread Willy Tarreau
nute breakage). Also, I know well enough that it's
sufficient to say that a version is the last -dev for about everyone to
skip it and wait for the final one! That's one more reason for not waiting
too long after it and not modifying it too much.

Please find the usual URLs below :
   Site index   : https://www.haproxy.org/
   Documentation: https://docs.haproxy.org/
   Wiki : https://github.com/haproxy/wiki/wiki
   Discourse: https://discourse.haproxy.org/
   Slack channel: https://slack.haproxy.org/
   Issue tracker: https://github.com/haproxy/haproxy/issues
   Sources  : https://www.haproxy.org/download/3.0/src/
   Git repository   : https://git.haproxy.org/git/haproxy.git/
   Git Web browsing : https://git.haproxy.org/?p=haproxy.git
   Changelog: https://www.haproxy.org/download/3.0/src/CHANGELOG
   Dataplane API: 
https://github.com/haproxytech/dataplaneapi/releases/latest
   Pending bugs : https://www.haproxy.org/l/pending-bugs
   Reviewed bugs: https://www.haproxy.org/l/reviewed-bugs
   Code reports : https://www.haproxy.org/l/code-reports
   Latest builds: https://www.haproxy.org/l/dev-packages

Willy
---
Complete changelog :
Amaury Denoyelle (20):
  BUG/MINOR: connection: parse PROXY TLV for LOCAL mode
  BUG/MINOR: server: free PROXY v2 TLVs on srv drop
  MINOR: rhttp: add log on connection allocation failure
  BUG/MEDIUM: rhttp: fix preconnect on single-thread
  BUG/MINOR: rhttp: prevent listener suspend
  BUG/MINOR: rhttp: fix task_wakeup state
  MINOR: session: define flag to explicitely release listener on free
  MEDIUM: rhttp: create session for active preconnect
  MINOR: rhttp: support PROXY emission on preconnect
  MINOR: connection: support PROXY v2 TLV emission without stream
  BUILD: trace: fix warning on null dereference
  MEDIUM: config: prevent communication with privileged ports
  MAJOR: config: prevent QUIC with clients privileged port by default
  BUG/MINOR: quic: adjust restriction for stateless reset emission
  MINOR: quic: clarify doc for quic_recv()
  MINOR: server: generalize sni expr parsing
  MINOR: server: define pool-conn-name keyword
  MEDIUM: connection: use pool-conn-name instead of sni on reuse
  BUG/MINOR: rhttp: initialize session origin after preconnect reversal
  DOC: quic: specify that connection migration is not supported

Aurelien DARRAGON (11):
  BUG/MINOR: ring: free ring's allocated area not ring's usable area when 
using maps
  DEBUG: tools: add vma_set_name() helper
  DEBUG: shctx: name shared memory using vma_set_name()
  DEBUG: sink: add name hint for memory area used by memory-backed sinks
  DEBUG: pollers: add name hint for large memory areas used by pollers
  DEBUG: errors: add name hint for startup-logs memory area
  DEBUG: fd: add name hint for large memory areas
  CLEANUP: tools: fix vma_set_name() function comment
  DEBUG: tools: add vma_set_name_id() helper
  DEBUG: pollers/fd: add thread id suffix to per-thread memory areas name 
hints
  BUG/MEDIUM: server/dns: preserve server's port upon resolution timeout or 
error

Christopher Faulet (8):
  BUG/MINOR: http-ana: Don't crush stream termination condition on internal 
error
  MAJOR: spoe: Let the SPOE back into the game
  BUG/MEDIUM: mux-quic: Create sedesc in same time of the QUIC stream
  MINOR: mux-quic: Set abort info for SC-less QCS on STOP_SENDING frame
  BUG/MEDIUM: stick-tables: Fix race with peers when trashing oldest entries
  BUG/MEDIUM: stick-tables: Fix race with peers when killing a sticky 
session
  BUG/MINOR: http-htx: Support default path during scheme based 
normalization
  BUG/MINOR: server: Don't reset resolver options on a new default-server 
line

Frederic Lecaille (1):
  BUG/MAJOR: quic: Crash with TLS_AES_128_CCM_SHA256 (libressl only)

Ilia Shipitsin (1):
  CI: scripts/build-ssl.sh: loudly fail on unsupported platforms

Valentine Krasnobaeva (4):
  BUG/MEDIUM: proto: fix fd leak in _connect_server
  MINOR: sock: set conn->err_code in case of EPERM
  BUG/MINOR: sock: fix sock_create_server_socket
  MINOR: proto: fix coding style

William Lallemand (9):
  CLEANUP: ssl/cli: remove unused code in dump_crtlist_conf
  MINOR: ssl: check parameter in ckch_conf_cmp()
  DOC: configuration: rework the crt-store load documentation
  MEDIUM: ssl: don't load file by discovering them in crt-store
  DOC: configuration: update the crt-list documentation
  DOC: configuration: add the supported crt-store options in crt-list
  REGTESTS: scripts: allow to change the vtest timeout
  CI: scripts/build-ssl: add a DESTDIR and TMPDIR variable
  CI: scripts/buil-ssl: cleanup the boringssl and quictls build

Willy Tarreau (12):
  MINOR: traces: enumerate the list of levels/verbosities when not found
  BUG/MEDIUM: q

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

2024-05-24 Thread Willy Tarreau
On Thu, May 23, 2024 at 03:58:45PM +0100, William Manley wrote:
> I can also report that I no longer need to avoid `nbthread 1` in the config
> on the node.  Presumably thanks to ceebb09744df367ad84586a341d9336f84f72bce
> "rhttp: fix preconnect on single-thread".

BTW keep in mind that connections may only be shared between threads
when they're idle; a connection with a request in flight on it will be
pinned to the thread that sent this request, and only other requests
coming from the same thread will be able to share this connection.

This means that you need to have enough pre-established connections to
be sure that in no case you have a thread that ends up with no available
connection. As such, if you want to keep the number of connections low,
it can make sense to limit the number of threads.

Willy



[ANNOUNCE] haproxy-3.0-dev12

2024-05-18 Thread Willy Tarreau
iple ckch_conf structures
  MEDIUM: ssl: temporarily load files by detecting their presence in 
crt-store
  REGTESTS: ocsp-update: change the reg-test to support the new crt-store 
mode

William Manley (1):
  MINOR: rhttp: Don't require SSL when attach-srv name parsing

Willy Tarreau (9):
  BUG/MEDIUM: htx: mark htx_sl as packed since it may be realigned
  BUG/MEDIUM: stick-tables: properly mark stktable_data as packed
  SCRIPTS: run-regtests: fix a few occurrences of extended regexes
  BUG/MINOR: ssl_sock: fix xprt_set_used() to properly clear the 
TASK_F_USR1 bit
  MINOR: dynbuf: provide a b_dequeue() variant for multi-thread
  BUG/MEDIUM: muxes: enforce buf_wait check in takeover()
  BUILD: stick-tables: better mark the stktable_data as 32-bit aligned
  CLEANUP: compat: make the MIN/MAX macros more reliable
  Revert: MEDIUM: evports: permit to report multiple events at once"

---



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

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

Excellent timing ;-)

Now merged, thank you Tim!
Willy



[ANNOUNCE] haproxy-3.0-dev11

2024-05-10 Thread Willy Tarreau
Hi,

HAProxy 3.0-dev11 was released on 2024/05/10. It added 43 new commits
after version 3.0-dev10.

Another very calm release, thanks to two days off here in France,
combined with the return of the sun today :-)

Some 3.0 regressions were fixed, such as the broken logs over TCP
after recent ring changes, and a QUIC crash when sending STOP_SENDING
after the stream above was detached.

The RST_STREAM reason code can finally be forwarded to the server for
client aborts. This addresses the problem a few users were facing with
gRPC where the abort reason was not respected. For now this is purposely
limited to only a few reason codes that are relevant to gRPC so that we
don't ruin the possibility to later extend that to H3 and maybe H1.

We also managed to address the design issue around the buffer_wait code
(the stuff that queues tasks waiting for a buffer on low memory). Now
tasks are queued by criticality depending on what buffer is missing, so
as to guarantee forward progress. It works pretty well for H1 and applets
already so that convinced me that we should finish it instead of dropping
all that. H2 and a few other places still need to be slightly revisited,
and I'd like to also merge a simplification that I experimented with that
allows the scheduler to deal itself with waiters instead of having every
user think about calling offer_buffers() when leaving. For now that part
was put to pause because doing it revealed another recent hard-to-trigger
regression that we'll work on early next week.

Finally, the build on Illumos was fixed, the log format options are now
processed at boot time to improve runtime processing performance, and as
usual, a few cleanups, doc and CI updates were included.

Overall this is getting good and I think we're on the right track for a
release around the end of the month.

Please find the usual URLs below :
   Site index   : https://www.haproxy.org/
   Documentation: https://docs.haproxy.org/
   Wiki : https://github.com/haproxy/wiki/wiki
   Discourse: https://discourse.haproxy.org/
   Slack channel: https://slack.haproxy.org/
   Issue tracker: https://github.com/haproxy/haproxy/issues
   Sources  : https://www.haproxy.org/download/3.0/src/
   Git repository   : https://git.haproxy.org/git/haproxy.git/
   Git Web browsing : https://git.haproxy.org/?p=haproxy.git
   Changelog: https://www.haproxy.org/download/3.0/src/CHANGELOG
   Dataplane API: 
https://github.com/haproxytech/dataplaneapi/releases/latest
   Pending bugs : https://www.haproxy.org/l/pending-bugs
   Reviewed bugs: https://www.haproxy.org/l/reviewed-bugs
   Code reports : https://www.haproxy.org/l/code-reports
   Latest builds: https://www.haproxy.org/l/dev-packages

Willy
---
Complete changelog :
Amaury Denoyelle (1):
  BUG/MEDIUM: mux-quic: fix crash on STOP_SENDING received without SD

Aurelien DARRAGON (3):
  OPTIM: log: resolve logformat options during postparsing
  BUG/MEDIUM: log/ring: broken syslog octet counting
  DOC: lua: fix filters.txt file location

Christopher Faulet (8):
  MEDIUM: stconn/muxes: Add an abort reason for SE shutdowns on muxes
  MINOR: mux-h2: Set the SE abort reason when a RST_STREAM frame is received
  MEDIUM: mux-h2: Forward h2 client cancellations to h2 servers
  MINOR: mux-quic: Set tha SE abort reason when a STOP_SENDING frame is 
received
  MINOR: stconn: Add samples to retrieve about stream aborts
  MINOR: mux-quic: Add .ctl callback function to get info about a mux 
connection
  MINOR: muxes: Add ctl commands to get info on streams for a connection
  MINOR: connection: Add samples to retrieve info on streams for a 
connection

Ilia Shipitsin (3):
  BUILD: clock: improve check for pthread_getcpuclockid()
  CI: add Illumos scheduled workflow
  CI: netbsd: limit scheduled workflow to parent repo only

Patrick Hemmer (3):
  REGTEST: add tests for acl() sample fetch
  BUG/MINOR: acl: support built-in ACLs with acl() sample
  BUG/MINOR: cfgparse: use curproxy global var from config post validation

Valentine Krasnobaeva (1):
  BUG/MINOR: haproxy: only tid 0 must not sleep if got signal

Willy Tarreau (24):
  MINOR: dynbuf: pass a criticality argument to b_alloc()
  MINOR: dynbuf: add functions to help queue/requeue buffer_wait fields
  MINOR: dynbuf: use the b_queue()/b_requeue() functions everywhere
  MEDIUM: dynbuf: make the buffer_wq an array of list heads
  CLEANUP: tinfo: better align fields in thread_ctx
  MINOR: dynbuf: provide a b_dequeue() function to detach a bw from the 
queue
  MEDIUM: dynbuf: generalize the use of b_dequeue() to detach buffer_wait
  MEDIUM: dynbuf/stream: re-enable queueing upon failed buffer allocation
  MEDIUM: dynbuf/stream: do not allocate the buffers in the callback
  MEDIUM: applet: make appctx_buf_available() only wake the applet up, not 
allocate
  MINOR

Re: error HAproxy with Galera Cluster v4

2024-05-10 Thread Willy Tarreau
Hello,

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

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

Willy



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

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

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

Willy



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

2024-05-06 Thread Willy Tarreau
Hi!

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

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

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

Thanks!
Willy



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

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

Cool, thanks, now applied.

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

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

Willy



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

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

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

Thanks,
Willy



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

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

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

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

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

Willy



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

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

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

Willy



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

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

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

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

Willy



[ANNOUNCE] haproxy-3.0-dev10

2024-05-04 Thread Willy Tarreau
 support rate in stats-file
  MINOR: stats: convert rate as generic column for proxy stats
  MINOR: counters: move last_change into counters struct
  MINOR: stats: support age in stats-file
  MINOR: stats: convert age as generic column for proxy stat
  REORG: stats: define stats-proxy source module
  MINOR: stats: extract proxy clear-counter in a dedicated function
  REGTESTS: stats: add test stats-file counters preload
  REGTESTS: replace REQUIRE_VERSION by version_atleast

Aurelien DARRAGON (15):
  CLEANUP: tools/cbor: rename cbor_encode_ctx struct members
  MINOR: log/cbor: _lf_cbor_encode_byte() explicitly requires non-NULL ctx
  BUG/MINOR: log: fix global lf_expr node options behavior
  CLEANUP: log: add a macro to know if a lf_node is configurable
  BUG/MINOR: log/encode: consider global options for key encoding
  BUG/MINOR: log/encode: fix potential NULL-dereference in LOGCHAR()
  BUG/MINOR: log: fix global lf_expr node options behavior (2nd try)
  MINOR: log/cbor: _lf_cbor_encode_byte() explicitly requires non-NULL ctx 
(again)
  BUG/MEDIUM: log: don't ignore disabled node's options
  MEDIUM: log: optimizing tmp->type handling in sess_build_logline()
  BUG/MINOR: log: prevent double spaces emission in sess_build_logline()
  OPTIM: log: declare empty buffer as global variable
  OPTIM: log: use thread local lf_buildctx to stop pushing it on the stack
  OPTIM: log: use lf_buildctx's buffer instead of temporary stack buffers
  OPTIM: log: speedup date printing in sess_build_logline() when no 
encoding is used

Ilia Shipitsin (2):
  CI: netbsd: adjust packages after NetBSD-10 released
  CLEANUP: assorted typo fixes in the code and comments

Remi Tricot-Le Breton (2):
  BUG/MEDIUM: cache: Vary not working properly on anything other than 
accept-encoding
  REGTESTS: cache: Add test on 'vary' other than accept-encoding

Valentine Krasnobaeva (7):
  MINOR: sock: rename sock to sock_fd in sock_create_server_socket
  MEDIUM: proto_uxst: take in account server namespace
  MEIDUM: unix sock: use my_socketat to create bind socket
  MINOR: sock_set_mark: take sock family in account
  MEDIUM: proto: make common fd checks in sock_create_server_socket
  MINOR: sock: add EPERM case in sock_handle_system_err
  MINOR: capabilities: add cap_sys_admin support

William Lallemand (6):
  MINOR: httpclient: allow to use absolute URI with new flag HC_F_HTTPROXY
  MINOR: ssl: introduce ocsp_update.http_proxy for ocsp-update keyword
  CLEANUP: ssl: clean the includes in ssl_ocsp.c
  CLEANUP: ssl: move the global ocsp-update options parsing to ssl_ocsp.c
  CLEANUP: ssl: rename new_ckch_store_load_files_path() to 
ckch_store_new_load_files_path()
  MINOR: ssl: rename ocsp_update.http_proxy into ocsp-update.httpproxy

Willy Tarreau (1):
  BUG/MINOR: stconn: don't wake up an applet waiting on buffer allocation

---



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

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

Both merged now, thank you Ilya!
Willy



Re: Question on deleting cookies from an HTTP request

2024-04-27 Thread Willy Tarreau
Hi,

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

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

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

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

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

Regards,
Willy



[ANNOUNCE] haproxy-3.0-dev9

2024-04-27 Thread Willy Tarreau
ction to know the side where an applet was created
  MEDIUM: peers: Simplify the peer flags dealing with the connection state
  MEDIUM: peers: Use true states for the peer applets as seen from outside
  MEDIUM: peers: Use true states for the learn state of a peer
  MINOR: peers: Start learning for local peer before receiving messages
  MINOR: peers: Rename PEERS_F_TEACH_COMPLETE to 
PEERS_F_LOCAL_TEACH_COMPLETE
  MINOR: peers: Reorder and slightly rename PEER flags
  MINOR: peers: Reorder and rename PEERS flags
  REORG: peers: Move peer and peers flags in the corresponding header file
  DEV: flags/peers: Decode PEER and PEERS flags
  MINOR: peers: Add comment on processing functions of the sync task
  MINOR: peers: Use a static variable to wait a resync on reload
  BUG/MEDIUM: peers: Use atomic operations on peers flags when necessary
  REORG: peers: Rename all occurrences to 'ps' variable
  BUG/MINOR: peers: Don't wait for a remote resync if there no remote peer

David Carlier (1):
  MEDIUM: shctx: Naming shared memory context

Remi Tricot-Le Breton (1):
  REGTESTS: ssl: Remove "sleep" calls from ocsp auto update test

Tim Duesterhus (3):
  MINOR: tools: Rename `ha_generate_uuid` to `ha_generate_uuid_v4`
  MINOR: Add `ha_generate_uuid_v7`
  MINOR: Add support for UUIDv7 to the `uuid` sample fetch

William Lallemand (3):
  BUILD: ssl: use %zd for sizeof() in ssl_ckch.c
  REGTESTS: use -dI for insecure fork by default in the regtest scripts
  BUG/MINOR: mworker: reintroduce way to disable seamless reload with -x 
/dev/null

Willy Tarreau (8):
  BUILD: stick-tables: silence build warnings when threads are disabled
  BUG/MINOR: h1: fix detection of upper bytes in the URI
  MINOR: intops: add a pair of functions to check multi-byte ranges
  TESTS: add a unit test for the multi-byte range checks
  CLEANUP: h1: make use of the multi-byte matching functions
  CLEANUP: dynbuf: move the reserve and limit parsers to dynbuf.c
  MINOR: list: add a macro to detect that a list contains at most one 
element
  MINOR: cli/wait: rename the condition "srv-unused" to "srv-removable"

---



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

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

Ah OK makes sense ;-)

Willy



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

2024-04-24 Thread Willy Tarreau
Hi!

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

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

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

Thanks!
Willy



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

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

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

Cheers,
Willy



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

2024-04-24 Thread Willy Tarreau
Hi David,

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

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

However I disagree with this point:

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

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

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

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

Thank you!
Willy



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

2024-04-24 Thread Willy Tarreau
Hi Tim!

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

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

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



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

2024-04-23 Thread Willy Tarreau
Hi Tim,

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

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

Thanks,
Willy



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

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

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

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

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

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

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

Willy



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

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

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

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

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

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

Willy



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

2024-04-19 Thread Willy Tarreau
Hi Tim!

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

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

Thanks and have a nice week-end!
Willy



[ANNOUNCE] haproxy-3.0-dev8

2024-04-19 Thread Willy Tarreau
ion endpoints
  MEDIUM: stconn: Explicitly pass shut modes to shut applet endpoints
  MEDIUM: stconn: Use one function to shut connection and applet endpoints
  MEDIUM: muxes: Use one callback function to shut a mux stream
  BUG/MEDIUM: peers: Don't set PEERS_F_RESYNC_PROCESS flag on a peer
  BUG/MEDIUM: peers: Fix state transitions of a peer

Damien Claisse (1):
  BUG/MINOR: server: fix slowstart behavior

Frederic Lecaille (2):
  MINOR: net_helper: Add support for floats/doubles.
  BUG/MEDIUM: grpc: Fix several unaligned 32/64 bits accesses

Ilya Shipitsin (4):
  CI: revert kernel addr randomization introduced in 3a0fc864
  CI: reduce ASAN log redirection umbrella size
  CLEANUP: assorted typo fixes in the code and comments
  CI: modernize macos matrix

Olivier Houchard (1):
  MINOR: stats: Get the right prototype for stats_dump_html_end().

Valentine Krasnobaeva (3):
  MINOR: listener/protocol: add proto name in alerts
  MINOR: proto_quic: add proto name in alert
  MINOR: init: use RLIMIT_DATA instead of RLIMIT_AS

William Lallemand (15):
  MINOR: ssl: add the section parser for 'crt-store'
  DOC: configuration: Add 3.12 Certificate Storage
  REGTESTS: ssl: test simple case of crt-store
  MINOR: ssl: rename ckchs_load_cert_file to new_ckch_store_load_files_path
  MINOR: ssl/crtlist: alloc ssl_conf only when a valid keyword is found
  CLEANUP: ssl: remove dead code in cfg_parse_crtstore()
  MINOR: ssl: supports crt-base in crt-store
  MINOR: ssl: 'key-base' allows to load a 'key' from a specific path
  MEDIUM: ssl: support aliases in crt-store
  BUG/MINOR: ssl: check on forbidden character on wrong value
  BUG/MINOR: ssl: fix crt-store load parsing
  MEDIUM: ssl: support a named crt-store section
  MEDIUM: ssl: crt-base and key-base local keywords for crt-store
  MAJOR: ssl: use the msg callback mecanism for backend connections
  MINOR: ssl: implement keylog fetches for backend connections

Willy Tarreau (43):
  BUG/MINOR: listener: always assign distinct IDs to shards
  BUILD: makefile: warn about unknown USE_* variables
  BUILD: makefile: support USE_xxx=0 as well
  BUILD: atomic: fix peers build regression on gcc < 4.7 after recent 
changes
  BUG/MINOR: debug: make sure DEBUG_STRICT=0 does work as documented
  BUILD: cache: fix non-inline vs inline declaration mismatch to silence a 
warning
  BUILD: debug: make DEBUG_STRICT=1 the default
  BUILD: pools: make DEBUG_MEMORY_POOLS=1 the default option
  CI: update the build options to get rid of unneeded DEBUG options
  BUILD: makefile: get rid of the config CFLAGS variable
  BUILD: makefile: allow to use CFLAGS to append build options
  BUILD: makefile: drop the SMALL_OPTS settings
  BUILD: makefile: move -O2 from CPU_CFLAGS to OPT_CFLAGS
  BUILD: makefile: get rid of the CPU variable
  BUILD: makefile: drop the ARCH variable and better document ARCH_FLAGS
  BUILD: makefile: extract ARCH_FLAGS out of LDFLAGS
  BUILD: makefile: move the fwrapv option to STD_CFLAGS
  BUILD: makefile: make the ERR variable also support 0
  BUILD: makefile: add FAILFAST to select the -Wfatal-errors behavior
  BUILD: makefile: extract -Werror/-Wfatal-errors from automatic CFLAGS
  BUILD: makefile: split WARN_CFLAGS from SPEC_CFLAGS
  BUILD: makefile: rename SPEC_CFLAGS to NOWARN_CFLAGS
  BUILD: makefile: do not pass warnings to VERBOSE_CFLAGS
  BUILD: makefile: also drop DEBUG_CFLAGS
  CLEANUP: makefile: make the output of the "opts" target more readable
  DOC: install: clarify the build process by splitting it into subsections
  BUG/MEDIUM: stick-tables: fix the task's next expiration date
  CLEANUP: stick-tables: always respect the to_batch limit when trashing
  BUG/MEDIUM: peers/trace: fix crash when listing event types
  BUG/MAJOR: stick-tables: fix race with peers in entry expiration
  DEBUG: pool: improve decoding of corrupted pools
  REORG: pool: move the area dump with symbol resolution to tools.c
  DEBUG: pools: report the data around the offending area in case of 
mismatch
  BUG/MINOR: lru: fix the standalone test case for invalid revision
  MINOR: ring: clarify the usage of ring_size() and add 
ring_allocated_size()
  BUG/MAJOR: ring: use the correct size to reallocate startup_logs
  MINOR: ring: always check that the old ring fits in the new one in 
ring_dup()
  BUILD: cache: fix a build warning with gcc < 7
  BUILD: xxhash: silence a build warning on Solaris + gcc-5.5
  BUG/MEDIUM: evports: do not clear returned events list on signal
  MEDIUM: evports: permit to report multiple events at once
  BUG/MINOR: sock: handle a weird condition with connect()
  BUG/MINOR: fd: my_closefrom() on Linux could skip contiguous series of 
sockets

---



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

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

Merged, thank you Ilya!
willy



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

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



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

2024-04-15 Thread Willy Tarreau
Hi Abhijeet,

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

You're welcome!

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

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

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

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

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

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

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

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

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

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

Thank you!
Willy



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

2024-04-15 Thread Willy Tarreau
Hi Abhijeet,

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

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

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

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

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

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

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

Ah, good point!

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

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

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

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

Thanks!
Willy



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

2024-04-13 Thread Willy Tarreau
Hi Tristan,

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

Definitely.

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Perfect, now merged, thank you Ilya!
Willy



Re: [PR] DOC: management: fix typos

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

Now merged, thank you Andrey!
Willy



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

2024-04-12 Thread Willy Tarreau
On Fri, Apr 12, 2024 at 12:42:51PM +0200,  ??? wrote:
> ping :)

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

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

Thanks,
Willy



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

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

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

Willy



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

2024-04-12 Thread Willy Tarreau
Hi!

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

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

Willy



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

2024-04-11 Thread Willy Tarreau
Hi Ilya,

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

That's this:

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

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

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

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

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

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

Cheers,
Willy



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

2024-04-11 Thread Willy Tarreau
Hi Damien,

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

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

It's now merged, thank you!
Willy



Changes in HAProxy 3.0's Makefile and build options

2024-04-11 Thread Willy Tarreau
Hi all,

after all the time where we've all been complaining about the difficulty
to adjust CFLAGS during the build, I could tackle the problem for a first
step in the right direction.

First, let's start with a bit of history to explain the situation and why
it was bad. Originally, a trivial makefile with very simple rules and a
single object to build (haproxy.o) had just CFLAGS set to "-O2 -g" or
something like that, and that was perfect. It was possible to just pass a
different CFLAGS value on the "make" command line and be done with it.

Options started to pile up, but in a way that remained manageable for a
long time (e.g. add PCRE support, later dlmalloc), so CFLAGS was still the
only thing to override if needed. With 32/64 bit variants and so on, we
started to feel the need to split those CFLAGS into multiple pieces for
more flexibility. But these pieces were still aggregated into CFLAGS so
that those used to overriding it were still happy. This situation forced
us not to break these precious CFLAGS that some users were relying on.

And that went like this for a long time, though the definition of this
CFLAGS variable became more and more complicated by inheriting from some
automatic options. For example, in 3.0-dev7, CFLAGS is initialized to
"$(ARCH_FLAGS) $(CPU_CFLAGS) $(DEBUG_CFLAGS) $(SPEC_CFLAGS)", i.e. it
concatenates 4 variables (in apparence). The 4th one (SPEC_CFLAGS) is
already a concatenation of a fixed one (WARN_CFLAGS) and a series of
automatically detected ones (the rest of it). ARCH_FLAGS is set from a
a fixed list of presets depending on the ARCH variable, and CPU_CFLAGS
is set from a list of presets depending on the CPU variable. And the
most beautiful is that CFLAGS alone is no longer sufficient since some
of the USE_* options append their own values behind it, and we complete
with $(TARGET_CFLAGS) $(SMALL_OPTS) $(DEFINE).

Yeah I know that's ugly. We all know it. Whenever someone asks me "how can
I enable -fsanitize=address because I'd like to run ASAN", I respond "hmmm
it depends what options you already use and which ones area easiest to
hack".

Some distros simply found that stuffing their regular CFLAGS into
DEBUG_CFLAGS or CPU_CFLAGS does the trick most of the time. Others use
other combinations depending on the tricks they figured.

After this analysis, I figured that we needed to simplify all this, get
back to what is more natural to use or more commonly found, without
breaking everything for everyone. What is certain however in the current
situation, is that nobody is overriding CFLAGS since it's too rich, too
complex and too unpredictable. So it was really time to reset it.

Thus here's what was done:
  - CFLAGS is now empty by default and can be passed some build options
that are appended at the end of the automatic flags. This means that
-Os, -ggdb3, -Wfoobar, -Wno-foobar, -I../secret-place/include etc
will properly be honored without having to trick anything anymore.
Thus for package maintainers, building with CFLAGS="$DISTRO_CFLAGS"
should just get the work done.

  - LDFLAGS is now empty by default and can be passed some build options
that are prepended to the list of linker options.

  - ARCH_FLAGS now defaults to -g and is passed to both the compiler and
the linker. It may be overridden to change the word size (-m32/-m64),
enable alternate debugging formats (-ggdb3), enable LTO (-flto),
ASAN (-fsanitize=address) etc. It's in fact for flags that must be
consistent between the compiler and the linker. It was not renamed
since it was already there and used quite a bit already.

  - CPU_CFLAGS was preserved for ease of porting but is empty by default.
Some may find it convenient to pass their -march, -mtune etc there.

  - CPU and ARCH are gone. Let's face it, only 2 of them were usable and
no single maintainer will be crazy enough to use these options here
and resort to another approach for other archs. However the previous
values are mentioned in the doc as a hint about what's known to work
well.

  - OPT_CFLAGS was created with "-O2" and nothing else. As developers,
we spend our time juggling between -O2 and -O0 depending on whether
we're testing or debugging. Some embedded distros also have options
to pass -O2 or -Os to choose between fast and small, and that may
fit very well there.

  - STD_CFLAGS contains a few flags related to standards compliance.
It is currently set to -fwrapv, -fno-strict-overflow and/or empty
depending on what the compiler prefers. It's important not to touch
it unless you know exactly what you're doing, and previously these
options used to be lost by accident when overriding other ones.

  - WARN_CFLAGS is now set to the list of warnings to be enabled. It's
discovered automatically but may be overridden without breaking the
rest.

  - NOWARN_CFLAGS is now set to the list of warnings to be disabled. It's
discovered automatically byt 

Re: [PR] Add destination ip as source ip

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

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

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

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

Willy



Re: [ANNOUNCE] haproxy-3.0-dev7

2024-04-08 Thread Willy Tarreau
Hi Ilya,

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

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

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

Cheers,
Willy



[ANNOUNCE] haproxy-3.0-dev7

2024-04-06 Thread Willy Tarreau
roxy.org/l/pending-bugs
   Reviewed bugs: https://www.haproxy.org/l/reviewed-bugs
   Code reports : https://www.haproxy.org/l/code-reports
   Latest builds: https://www.haproxy.org/l/dev-packages

Willy
---
Complete changelog :
Amaury Denoyelle (8):
  BUG/MINOR: server: reject enabled for dynamic server
  MINOR: server: allow cookie for dynamic servers
  BUG/BUILD: debug: fix unused variable error
  MINOR: guid: introduce global UID module
  MINOR: guid: restrict guid format
  MINOR: proxy: implement GUID support
  MINOR: server: implement GUID support
  MINOR: listener: implement GUID support

Anthony Deschamps (1):
  MEDIUM: lb-chash: Deterministic node hashes based on server address

Aurelien DARRAGON (14):
  DOC: config: balance 'first' not usable in LOG mode
  BUG/MINOR: log/balance: detect if user tries to use unsupported algo
  MINOR: lbprm: implement true "sticky" balance algo
  MEDIUM: log/balance: leverage lbprm api for log load-balancing
  BUG/MEDIUM: server/lbprm: fix crash in _srv_set_inetaddr_port()
  BUG/MINOR: proxy: fix logformat expression leak in use_backend rules
  MEDIUM: log: rename logformat var to logformat tag
  MINOR: log: expose logformat_tag struct
  MEDIUM: log: carry tag context in logformat node
  MEDIUM: tree-wide: add logformat expressions wrapper
  MINOR: proxy: add PR_FL_CHECKED flag
  MAJOR: log: implement proper postparsing for logformat expressions
  MEDIUM: log: add compiling logic to logformat expressions
  MEDIUM: proxy/log: leverage lf_expr API for logformat preparsing

Christopher Faulet (14):
  REGTESTS: Fix script about OCSP update compatibility tests
  BUG/MINOR: cli: Report an error to user if command or payload is too big
  MINOR: sc_strm: Add generic version to perform sync receives and sends
  MEDIUM: stream: Use generic version to perform sync receives and sends
  MEDIUM: buf: Add b_getline() and b_getdelim() functions
  MEDIUM: applet: Handle applets with their own buffers in put functions
  MEDIUM: cli/applet: Stop to test opposite SC in I/O handler of CLI 
commands
  MINOR: applet: Always use applet API to set appctx flags
  BUG/MEDIUM: applet: State appctx have more data if its EOI/EOS/ERROR flag 
is set
  MAJOR: cli: Update the CLI applet to handle its own buffers
  MINOR: applet: Let's applets .snd_buf function deal with full input 
buffers
  MINOR: stconn: Add a connection flag to notify sending data are the last 
ones
  MAJOR: cli: Use a custom .snd_buf function to only copy the current 
command
  BUG/MEDIUM: stconn: Don't forward shutdown to SE if iobuf is not empty

Damien Claisse (1):
  BUG/MINOR: server: fix persistence cookie for dynamic servers

Frederic Lecaille (3):
  MINOR: quic: HyStart++ implementation (RFC 9406)
  BUILD: quic: 32 bits compilation issue (QUIC_MIN() usage)
  BUG/MINOR: stick-tables: Missing stick-table key nullity check

Ilya Shipitsin (2):
  CI: vtest: show coredumps if any
  CI: extend Fedora Rawhide, add m32 mode

Nicolas CARPi (1):
  DOC: configuration: grammar fixes for strict-sni

Remi Tricot-Le Breton (5):
  BUG/MINOR: ssl: Wrong ocsp-update "incompatibility" error message
  BUG/MINOR: ssl: Detect more 'ocsp-update' incompatibilities
  MEDIUM: ssl: Add 'tune.ssl.ocsp-update.mode' global option
  REGTESTS: ssl: Add OCSP update compatibility tests
  REGTESTS: ssl: Add functional test for global ocsp-update option

Tim Duesterhus (7):
  REGTESTS: Do not use REQUIRE_VERSION for HAProxy 2.5+ (4)
  REGTESTS: Remove REQUIRE_VERSION=1.9 from all tests (2)
  CLEANUP: Reapply ist.cocci (3)
  CLEANUP: Reapply strcmp.cocci (2)
  CLEANUP: Reapply xalloc_cast.cocci
  CLEANUP: Reapply ha_free.cocci
  MINOR: systemd: Include MONOTONIC_USEC field in RELOADING=1 message

Valentine Krasnobaeva (3):
  BUG/MINOR: init: relax LSTCHK_NETADM checks for non root
  MEDIUM: capabilities: check process capabilities sets
  CLEANUP: global: remove LSTCHK_CAP_BIND

William Lallemand (3):
  REGTESTS: ssl: disable ssl/ocsp_auto_update.vtc
  MEDIUM: mworker: get rid of libsystemd
  BUILD: systemd: enable USE_SYSTEMD by default with TARGET=linux-glibc

Willy Tarreau (11):
  BUG/MEDIUM: stick-tables: fix a small remaining race in expiration task
  BUG/MINOR: backend: properly handle redispatch 0
  BUG/MEDIUM: stick-table: use the update lock when reading tables from 
peers
  BUG/MAJOR: applet: fix a MIN vs MAX usage in appctx_raw_rcv_buf()
  OPTIM: peers: avoid the locking dance around 
peer_send_teach_process_msgs()
  BUILD: systemd: fix build error on non-systemd systems with USE_SYSTEMD=1
  BUG/MINOR: bwlim/config: fix missing '\n' after error messages
  MINOR: stick-tables: mark the seen stksess with a flag "seen"
  OPTIM: stick-tables

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

2024-04-05 Thread Willy Tarreau
Hi Bertrand!

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

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

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

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

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

Cheers,
Willy



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

2024-04-05 Thread Willy Tarreau
Hi Nicolas,

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

Now merged, thank you!
Willy



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

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

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



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

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

Even better :-)

Thanks David!
Willy



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

2024-04-02 Thread Willy Tarreau
Hello,

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

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

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

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

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

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

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

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

Hoping this helps,
Willy



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

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

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

Willy



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

2024-04-01 Thread Willy Tarreau
Hi Tim!

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

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



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

2024-04-01 Thread Willy Tarreau
Hi Anthony,

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

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



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

2024-03-29 Thread Willy Tarreau
Hi Ricardo,

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

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

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

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

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

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

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

OK!

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

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

Willy



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

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

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

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

Cheers,
Willy



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

2024-03-27 Thread Willy Tarreau
Hi again Anthony,

I'm still having a few comments, but I think nothing that I cannot address
while merging it:

On Wed, Mar 13, 2024 at 12:33:54PM -0400, Anthony Deschamps wrote:
> +static inline u32 chash_compute_server_key(struct server *s)
> +{
> + u32 key = 0;
> + struct server_inetaddr srv_addr;
> +
> + enum srv_hash_key hash_key = s->hash_key;

In general, it's better to prefer the common "reverse christmas tree"
declaration order, i.e. put larger statements first and smaller next,
and more importantly not leave empty lines in the middle of declarations
as these usually indicate the beginning of statements.

> +
> + /* If hash-key is addr or addr-port then we need the address, but if we
> +  * can't determine the address then we fall back on hashing the puid.
> +  */
> + switch (s->hash_key) {

Here I initially got confused by the use of s->hash_key while the rest
below uses hash_key, I'd change it to hash_key as well since it's already
assigned at the top.

> + case SRV_HASH_KEY_ADDR:
> + case SRV_HASH_KEY_ADDR_PORT:
> + server_get_inetaddr(s, _addr);
> + if (srv_addr.family != AF_INET && srv_addr.family != AF_INET6) {
> + hash_key = SRV_HASH_KEY_ID;
> + }
> + break;
> +
> + default:
> + break;
> + }
> +
> + if (hash_key == SRV_HASH_KEY_ADDR_PORT) {
> + key = full_hash(srv_addr.port.svc);
> + }

Given that "key" is used immediately below and that I had to scroll up a
bit to verify that it was properly initialized, I'd rather set the default
zero value here in an else statement for ease of inspection later.

> + switch (hash_key) {
> + case SRV_HASH_KEY_ADDR_PORT:
> + case SRV_HASH_KEY_ADDR:
> + switch (srv_addr.family) {
> + case AF_INET:
> + key = full_hash(key + srv_addr.addr.v4.s_addr);
> + break;
> + case AF_INET6:
> + key = XXH32(srv_addr.addr.v6.s6_addr, 16, key);
> + break;
> + default:
(...)

> +  Arguments :
> +may be one of the following :
> +
> +  id The node keys will be derived from the server's position in
> + the server list.
> +

Here it's "the server's numeric identifier as set from 'id' or which
defaults to its position in the list".

The rest is good.

If you want I can perform these tiny cosmetic adjustments myself so as
to save you a round trip.

Thanks!
Willy



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

2024-03-27 Thread Willy Tarreau
Hi Anthony,

On Sun, Mar 24, 2024 at 10:11:41PM -0400, Anthony Deschamps wrote:
> Hi Willy,
> 
> I'm just checking in to see if there's anything left I can help address here.

Thanks for the ping and sorry for the delay. It just fell through the
cracks in the middle of all other stuff I'm currently having to deal
with in parallel, as every time we're starting to get closer to a
release :-(

I'll try to deal with it today.

Willy



[ANNOUNCE] haproxy-3.0-dev6

2024-03-26 Thread Willy Tarreau
pository   : https://git.haproxy.org/git/haproxy.git/
   Git Web browsing : https://git.haproxy.org/?p=haproxy.git
   Changelog: https://www.haproxy.org/download/3.0/src/CHANGELOG
   Dataplane API: 
https://github.com/haproxytech/dataplaneapi/releases/latest
   Pending bugs : https://www.haproxy.org/l/pending-bugs
   Reviewed bugs: https://www.haproxy.org/l/reviewed-bugs
   Code reports : https://www.haproxy.org/l/code-reports
   Latest builds: https://www.haproxy.org/l/dev-packages

Willy
---
Complete changelog :
Amaury Denoyelle (13):
  MINOR: quic: simplify rescheduling for handshake
  MINOR: quic: remove qc_treat_rx_crypto_frms()
  MINOR: session: rename private conns elements
  BUG/MAJOR: server: do not delete srv referenced by session
  BUG/MINOR: session: ensure conn owner is set after insert into session
  BUG/MEDIUM: http_ana: ignore NTLM for reuse aggressive/always and no H1
  BUG/MAJOR: connection: fix server used_conns with H2 + reuse safe
  MINOR: connection: implement conn_release()
  MINOR: connection: extend takeover with release option
  MEDIUM: server: close idle conn on server deletion
  MEDIUM: mux: prepare for takeover on private connections
  MEDIUM: server: close private idle connection before server deletion
  BUG/MINOR: mux-quic: close all QCS before freeing QCC tasklet

Aurelien DARRAGON (9):
  BUG/MINOR: hlua: segfault when loading the same filter from different 
contexts
  BUG/MINOR: hlua: missing lock in hlua_filter_new()
  BUG/MINOR: hlua: fix missing lock in hlua_filter_delete()
  DEBUG: lua: precisely identify if stream is stuck inside lua or not
  MINOR: hlua: use accessors for stream hlua ctx
  BUG/MEDIUM: hlua: streams don't support mixing lua-load with 
lua-load-per-thread (2nd try)
  BUILD: server: fix build regression on old compilers (<= gcc-4.4)
  OPTIM: http_ext: avoid useless copy in http_7239_extract_{ipv4,ipv6}
  BUG/MINOR: server: 'source' interface ignored from 'default-server' 
directive

Brooks Davis (1):
  MINOR: tools: use public interface for FreeBSD get_exec_path()

Christopher Faulet (9):
  BUG/MINOR: listener: Wake proxy's mngmt task up if necessary on session 
release
  BUG/MINOR: listener: Don't schedule frontend without task in 
listener_release()
  BUG/MEDIUM: spoe: Don't rely on stream's expiration to detect processing 
timeout
  BUG/MINOR: spoe: Be sure to be able to quickly close IDLE applets on 
soft-stop
  MAJOR: spoe: Deprecate the SPOE filter
  MINOR: cfgparse: Add a global option to expose deprecated directives
  MINOR: spoe: Add SPOE filters in the exposed deprecated directives
  BUG/MEDIUM: spoe: Return an invalid frame on recv if size is too small
  BUG/MEDIUM: mux-fcgi: Properly handle EOM flag on end-of-trailers HTX 
block

Dragan Dosen (2):
  BUG/MINOR: ssl: fix possible ctx memory leak in sample_conv_aes_gcm()
  BUG/MINOR: ssl: do not set the aead_tag flags in sample_conv_aes_gcm()

Ilia Shipitsin (2):
  CLEANUP: assorted typo fixes in the code and comments
  CI: temporarily adjust kernel entropy to work with ASAN/clang

Remi Tricot-Le Breton (8):
  BUG/MAJOR: ocsp: Separate refcount per instance and per store
  REGTESTS: ssl: Add OCSP related tests
  BUG/MEDIUM: ssl: Fix crash when calling "update ssl ocsp-response" when 
an update is ongoing
  BUG/MEDIUM: ssl: Fix crash in ocsp-update log function
  MEDIUM: ssl: Change output of ocsp-update log
  MINOR: ssl: Change level of ocsp-update logs
  CLEANUP: ssl: Remove undocumented ocsp fetches
  REGTESTS: ssl: Add checks on ocsp-update log format

William Lallemand (7):
  DOC: configuration: clarify ciphersuites usage (V2)
  MEDIUM: ssl: initialize the SSL stack explicitely
  MEDIUM: ssl: allow to change the OpenSSL security level from global 
section
  CLEANUP: ssl: remove useless #ifdef in openssl-compat.h
  CI: github: add -DDEBUG_LIST to the default builds
  MINOR: debug: enable insecure fork on the command line
  CI: github: add -dI to haproxy arguments

Willy Tarreau (63):
  MINOR: mux-h2: always use h2c_report_glitch()
  MEDIUM: mux-h2: allow to set the glitches threshold to kill a connection
  BUG/MINOR: server: fix first server template not being indexed
  MINOR: debug: add "debug dev trace" to flood with traces
  MINOR: atomic: add a read-specific variant of __ha_cpu_relax()
  MINOR: applet: add new function applet_append_line()
  MINOR: log/applet: add new function syslog_applet_append_event()
  MEDIUM: ring/sink: use applet_append_line()/syslog_applet_append_event() 
for readers
  REORG: dns/ring: split the ring between the generic one and the DNS one
  MEDIUM: ring: move the ring reader code to ring_dispatch_messages()
  MEDIUM: sink: move the generic ring forwarder code use 
ring_di

Re: About the SPOE

2024-03-25 Thread Willy Tarreau
Hi Lokesh,

On Tue, Mar 26, 2024 at 12:10:53AM +, Lokesh Jindal wrote:
> Hey Willy
> 
> Resending this email in case you missed the last one. Let me know if you had
> any follow up questions/comments.
> I saw https://github.com/haproxy/haproxy/issues/2502 created by Christopher -
> looking forward to discussion on that issue.

Sorry for the delay, yes I had a quick look but last week was full of
meetings here and I'm slowly trying to catch up with the lag, I'm even
late on -dev6 :-(  Back on this soon.  There could be good news from
our side given that some of the meetings were precisely about all of
this ;-)

Stay tuned,
Willy



Re: [PR] FEATURE: load private keys from PKCS#11 pkcs11-provider PEM files

2024-03-21 Thread Willy Tarreau
On Thu, Mar 21, 2024 at 10:58:17AM +0100, William Lallemand wrote:
> On Thu, Mar 21, 2024 at 05:34:12PM +0800, Richard Chan wrote:
> > Yes I would be happy to include HAProxy with pkcs11-provider examples.
> > 
> 
> Great, thank you!
> 
> I made a `PKCS11 provider` 
> https://github.com/haproxy/wiki/wiki/OpenSSL-Providers-in-HAProxy#pkcs11-provider
> that you could edit once we give you the rights.
> 
> Willy: Could you give the right to edit this page to the `space88man`
> github user? Thanks!

Sute! It should be OK now, an invite was sent.

Willy



Re: [PATCH 0/2] CI entropy adjust (clang asan fix) and spell fixes

2024-03-18 Thread Willy Tarreau
On Sun, Mar 17, 2024 at 05:01:37PM +0100, Ilia Shipitsin wrote:
> couple of patches
> 1) spell fixes
> 2) CI sysctl to make new ubuntu kernels and asan friends again

Now merged, thanks for dealing with this Ilya. I understood from the
GH issue that we can hope to remove it by the end of this week, but
at least if it can help avoid triggering dummy bugs, that will be a
progress!

Willy



Re: About the SPOE

2024-03-18 Thread Willy Tarreau
Hi Lokesh, Abhijeet, Alex,

First, thanks for jumping into this thread, the purpose of the
deprecation is in a big part to try to collect the requirements
of possibly existing users. Mind you that the rare times we hear
about SPOE is only because of problems, so it's difficult to figure
what to keep and what to cut from the existing design.

More on that below:

On Fri, Mar 15, 2024 at 05:15:14PM +, Lokesh Jindal wrote:
> Hey Christopher
> 
> Adding to what my colleague, Abhijeet, said.
> 
> 
>   1.  We plan to ramp traffic to HAProxy very soon where we will heavily rely
>   on SPOA. In our testing, we are satisfied with SPOE in terms of
>   performance. The flexibility to write SPOA in any language not only allows
>   us to handle "complex use cases" like connecting to non-http downstreams,
>   but also helps in observability - metrics and logging.

Interesting, the initial internal e-mail in 2016 that ignited the SPOE
design was driven from the same observations: in web environments it's
quite rare to find developers who are at ease with system-level languages,
yet they are the ones most likely to request to extend the proxy. For
this reason we wanted to offer the possibility to call code written in
other languages. In addition it was estimated that the ability to connect
to the agent over the network and using secure connections was absolutely
essential. It brings the ability to scale the processing engine without
adding more LB nodes, and even to offload that to another DC, infrastructure
or even to delegate it to a 3rd party.

Among the use cases which immediately came to mind were authentication,
database accesses, name resolution, "remote maps", request classification,
IP reputation, etc. In addition we thought that limiting ourselves to short
request/responses like this was probably limiting and that it would have
been useful to support streaming so that we could implement image
recompression, caching, WAFs etc.

The first PoC implementation that was merged in version 1.7 lacked the
streaming abilities, and it's still the current implementation. It took
a while before we received feedback on it, since then caching was
implemented, the demand for image recompression is close to non-existing,
and WAFs users have well accommodated to dealing with extra layers by now
it seems. So basically we're left with something ultra-complex that deals
with short request-responses most of the time, and that suffers from the
initial design which was way bigger than the use cases.

>   2.  What is the best alternative to SPOE?

I don't know yet, and the purpose of this deprecation precisely is to
engage discussion with users. One could think about various HTTP-based
protocols for which it is easier to implement a server, some gRPC maybe,
possibly even a stripped-down version of the SPOP protocol if we figure
that everyone is running on a reasonable subset that's much easier to
deal with than the whole stuff we have now.

> Two options that we are aware of
>   - Write fetchers/converters in lua or write filters in other languages
>   using the Lua API. In your experience, how do they compare to SPOE in terms
>   of:
>  *   Performance
>  *   Fault isolation

The benefits of SPOE as you've found, clearly are in terms of flexibility
as it allows to scale the number of analysers independently on the number
of LBs, and it almost makes problems almost unnoticeable. For example, if
your code relied on unstable 3rd party libraries, crashing your SPOA doesn't
bring down the whole proxy. Similarly in terms of added latency, all the
latency is in the external component, the rest of the traffic is not
affected as it would be by placing some heavy processing directly inside
the haproxy process.

>   3.  As Abhijeet said, can you share a list of issues with SPOE that make it
>   hard to maintain?

On the top of my head I can enumerate a few:

  - the load balancing is integraly reimplemented at the SPOE level
between applets

  - the load balancing is affected by the support of response-less
messages, which prevent haproxy from having an estimate of the
server's load, which means that an algo such as leastconn would
not make sense in such a condition

  - the idle connections management is integraly reimplemented at
the SPOE level using applets as well. An idle SPOE connection is
in fact a full application-layer stream with one applet on one
side and a connection on the other side. These cannot be migrated
between threads, which require even more complex stream-to-stream
thread-safe communication mechanisms and synchronisation.

  - it's possible to receive a response on a different connection
than the one that saw the request. This adds complexity in the
matching request lookup, needs for extra synchronisation so that
the other stream doesn't vanish while we're delivering the
reponse, and it also makes it harder to keep track of the number
of in-flight requests. 

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

2024-03-13 Thread Willy Tarreau
Hi Anthony,

On Wed, Mar 13, 2024 at 12:33:54PM -0400, Anthony Deschamps wrote:
> Hi Willy,
> 
> My original concern was that if two servers had values of lb_server_key
> that are close to each other, then there could be some overlap in their
> values of lb_server_key + node_index;, which is why I was looking for a
> reasonable hash function to combine them.

I know but you can't really avoid it anyway, and that's why we place
many nodes. I tried an attempt in the past where nodes were evenly
spaced by dividing by the number of entries etc. It was a disaster.
As soon as a server went down, the next one would take its extra load
for example. So anything you try to do to avoid some corner cases create
other ones, while overall, a random distribution (like a hash does)
gives great results.

> I based it on boost::hash_combine,
> but since writing that I came across https://stackoverflow.com/a/50978188
> explaining why boost::hash_combine isn't necessarily a good hash function.

Hehe. We've all written our own hash functions for a given purpose at one
point in time, and they're usually fine inside their context and bad once
used outside. That's also where XXH and such stuff shines, it brings
generic functions that are good all over the space. In our case, the
full_hash() is just an avalanche non-linear 1:1 mapping between the input
and the output which suffices to avoid resonance effects that come from
repeated additions, it's perfectly suited here I think.

> I agree that it's not too critical to have a few collisions, but I'm reaching
> the limits of my knowledge here. Does this change address your concerns?
> I've attached an updated patch, and here's the diff between them:
> 
> diff --git a/src/lb_chash.c b/src/lb_chash.c
> index c77222854..d7a1b2539 100644
> --- a/src/lb_chash.c
> +++ b/src/lb_chash.c
> @@ -66,9 +66,7 @@ static inline void chash_dequeue_srv(struct server *s)
>   */
>  static inline u32 chash_compute_node_key(struct server *s, unsigned 
> node_index)
>  {
> -u32 server_key = s->lb_server_key;
> -u32 index_key = full_hash(node_index);
> -return server_key ^= index_key + 0x9e3779b9 + (server_key << 6) +
> (server_key >> 2);
> +return full_hash(s->lb_server_key + node_index);
>  }
> 
>  /* Compute the base server key that will be used to compute node keys. 
> Servers
> @@ -112,8 +110,7 @@ static inline u32 chash_compute_server_key(struct server 
> *s)
>  case SRV_HASH_KEY_ADDR:
>  switch (srv_addr.family) {
>  case AF_INET:
> -u32 addr_key = full_hash(srv_addr.addr.v4.s_addr);
> -key ^= addr_key + 0x9e3779b9 + (key << 6) + (key >> 2);
> +key = full_hash(key + srv_addr.addr.v4.s_addr);
>  break;
>  case AF_INET6:
>  key = XXH32(srv_addr.addr.v6.s6_addr, 16, key);

Yeay I think it addresses everything. I'll re-test your updated patch
tomorrow hoping that this time I'll merge it :-)

Thanks for your patience!
Willy



Re: Fix haproxy build on recent FreeBSD

2024-03-13 Thread Willy Tarreau
On Mon, Mar 11, 2024 at 07:00:26PM +0100, Willy Tarreau wrote:
> On Mon, Mar 11, 2024 at 05:56:35PM +, Brooks Davis wrote:
> > > OK that works for me. Do you want to send a new patch or should I adapt
> > > yours ? If you have a 12 somewhere that would save me time to verify I
> > > don't mess up with the ifdefs, otherwise I can probably handle it and
> > > we'll watch for any report of build error.
> > 
> > I don't have a 12 box handy either, but here's a patch that retains the
> > old code wrapped in ifdefs and should be low risk as such.  1300058 is
> > the next version bump after AT_EXECPATH support was added.
> 
> Much appreciated, thank you Brooks! I know someone with a 12 willing to
> test (even though your patch should not break anything as-is).

And FWIW I got the confirmation this morning that it still works fine
on 12 ;-)

Willy



Re: Revisiting CVE-2023-45539

2024-03-13 Thread Willy Tarreau
Hi Ryan,

On Mon, Mar 04, 2024 at 12:13:48PM -0600, Ryan O'Hara wrote:
> I am looking at CVE-2023-45539 as it affects older versions of haproxy (ie.
> haproxy-1.8). At this point I have verified that 1.8 is affected by this
> issue, which is in agreement with the original bug/commit which states
> versions prior to 2.8 need a backport.

So apparently this is about this one:

  2eab6d3543 ("BUG/MINOR: h1: do not accept '#' as part of the URI component")

> I am wondering if anyone has
> attempted or completed this backport. I am happy to provide one with the
> understanding that this will not be merged as 1.8 is EOL.

Not attempted, but I had a quick look, and it seems to me that the part
for 2.0 that's related to the legacy code should apply well and work,
since there are very few differences between 1.8 and 2.0 regarding this.
The part that applies to HTX just has to be dropped.

Well, finally I tried and it was not hard, I could even validate that
it works using the regtest.

I'm attaching the following backported patches, only the first 3 are
needed, the remaining ones are for h2, but since in 1.8 h2 is turned
into h1, you probably don't need them anyway.

Hoping this helps,
Willy
>From 4e98c0c1d36104ed426d3b198a176e1a5df814fa Mon Sep 17 00:00:00 2001
From: Willy Tarreau 
Date: Tue, 8 Aug 2023 16:17:22 +0200
Subject: BUG/MINOR: h1: do not accept '#' as part of the URI component

Seth Manesse and Paul Plasil reported that the "path" sample fetch
function incorrectly accepts '#' as part of the path component. This
can in some cases lead to misrouted requests for rules that would apply
on the suffix:

use_backend static if { path_end .png .jpg .gif .css .js }

Note that this behavior can be selectively configured using
"normalize-uri fragment-encode" and "normalize-uri fragment-strip".

The problem is that while the RFC says that this '#' must never be
emitted, as often it doesn't suggest how servers should handle it. A
diminishing number of servers still do accept it and trim it silently,
while others are rejecting it, as indicated in the conversation below
with other implementers:

   https://lists.w3.org/Archives/Public/ietf-http-wg/2023JulSep/0070.html

Looking at logs from publicly exposed servers, such requests appear at
a rate of roughly 1 per million and only come from attacks or poorly
written web crawlers incorrectly following links found on various pages.

Thus it looks like the best solution to this problem is to simply reject
such ambiguous requests by default, and include this in the list of
controls that can be disabled using "option accept-invalid-http-request".

We're already rejecting URIs containing any control char anyway, so we
should also reject '#'.

In the H1 parser for the H1_MSG_RQURI state, there is an accelerated
parser for bytes 0x21..0x7e that has been tightened to 0x24..0x7e (it
should not impact perf since 0x21..0x23 are not supposed to appear in
a URI anyway). This way '#' falls through the fine-grained filter and
we can add the special case for it also conditionned by a check on the
proxy's option "accept-invalid-http-request", with no overhead for the
vast majority of valid URIs. Here this information is available through
h1m->err_pos that's set to -2 when the option is here (so we don't need
to change the API to expose the proxy). Example with a trivial GET
through netcat:

  [08/Aug/2023:16:16:52.651] frontend layer1 (#2): invalid request
backend  (#-1), server  (#-1), event #0, src 127.0.0.1:50812
buffer starts at 0 (including 0 out), 16361 free,
len 23, wraps at 16336, error at position 7
H1 connection flags 0x, H1 stream flags 0x0810
H1 msg state MSG_RQURI(4), H1 msg flags 0x1400
H1 chunk len 0 bytes, H1 body len 0 bytes :

0  GET /aa#bb HTTP/1.0\r\n
00021  \r\n

This should be progressively backported to all stable versions along with
the following patch:

REGTESTS: http-rules: add accept-invalid-http-request for normalize-uri 
tests

Similar fixes for h2 and h3 will come in followup patches.

Thanks to Seth Manesse and Paul Plasil for reporting this problem with
detailed explanations.

(cherry picked from commit 2eab6d354322932cfec2ed54de261e4347eca9a6)
Signed-off-by: Amaury Denoyelle 
(cherry picked from commit 9bf75c8e22a8f2537f27c557854a8803087046d0)
Signed-off-by: Amaury Denoyelle 
(cherry picked from commit 9facd01c9ac85fe9bcb331594b80fa08e7406552)
Signed-off-by: Amaury Denoyelle 
(cherry picked from commit 832b672eee54866c7a42a1d46078cc9ae0d544d9)
Signed-off-by: Willy Tarreau 
(cherry picked from commit e5a741f94977840c58775b38f8ed830207f7e4d0)
Signed-off-by: Willy Tarreau 
(cherry picked from commit 178cea76b1c9d9413afa6961b6a4576fcb5b26fa)
[wt: applied the same to http_parse_reqline() in http_msg.c]
Signed-off-by: Willy Tarreau 
(cherry picked from commit 4ad6fd9eeb3078685fffdc58f1c6d4eb97e05d98)
[wt: d

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

2024-03-13 Thread Willy Tarreau
Hi Anthony,

On Tue, Mar 12, 2024 at 07:10:42PM -0400, Anthony Deschamps wrote:
> Hi Willy,
> 
> Thanks for the feedback. I had been testing with smaller numbers of
> servers (usually between 4 and 32) so I hadn't noticed the performance
> impact.

Not surprised. I'm used to testing with some extreme setups for knowing
such exist in field (more than 1000 servers in a single backend were
already reported for example).

> Here's an updated patch (as an attachment, until I figure out how
> to make sure things are formatted correctly!) that should address that.

Thanks. It's not a problem attached this way because my mailer still
recognizes it as text and inlines it when responding, so that doesn't
alter my ability to comment.

> I added a "lb_server_key" to the server struct. I moved most of the hashing
> logic that I previously had in "chash_compute_node_key" into a new function,
> "chash_compute_server_key", and I store that value; nodes only need to be
> removed and reinserted if this value changes. To compute a node key, I just
> combine the server key with a hash of node_index. I considered using
> XXH32_round to do this, but that looks like it's intended to be an internal
> function, so I inlined something similar instead.

You could always decide to combine an existing hash with another value, or
combine one 32-bit key with a 32-bit index and apply a 64-bit hash, but I
think that's fine this way.

> Hopefully this all works. Thanks for taking the time to work through this, and
> please let me know if there's anything else to address.

I just got this:

  src/lb_chash.c: In function 'chash_compute_server_key':
  src/lb_chash.c:115:4: error: a label can only be part of a statement and a 
declaration is not a statement
  compilation terminated due to -Wfatal-errors.

That's because of this:

> + switch (hash_key) {
> + case SRV_HASH_KEY_ADDR_PORT:
> + case SRV_HASH_KEY_ADDR:
> + switch (srv_addr.family) {
> + case AF_INET:
> + u32 addr_key = full_hash(srv_addr.addr.v4.s_addr);

Here that's a declaration after a statement ("case" not being considered
as the beginning of a block since you can pretty well fall through from
a previous one).

I'll adjust it while merging.

I had a concern about the hashing function which seems to reduce the
input key space, and indeed it does, it reduces 4294967296 server keys
to only 2948667289 (68.65 %):

> + key ^= addr_key + 0x9e3779b9 + (key << 6) + (key >> 2);

Not critical but it would be better if we can avoid this loss. I tried
with the full_hash() function and it preserves all output keys. So I
think you should instead use:

key = full_hash(add_key);

and chash_compute_node_key() could then simply be achieved this way:

  static inline u32 chash_compute_node_key(struct server *s, unsigned 
node_index)
  {
   return full_hash(s->lb_server_key + node_index);
  }

Other than that, I'm OK with the rest.

Thanks!
Willy



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

2024-03-12 Thread Willy Tarreau
Hi again Anthony,

First comment, thanks for the patch, it's of very good quality!
I'm having *one* problem with it, below:

>  /* Adjust the number of entries of a server in its tree. The server must 
> appear
>   * as many times as its weight indicates it. If it's there too often, we 
> remove
>   * the last occurrences. If it's not there enough, we add more occurrences. 
> To
> @@ -67,39 +126,32 @@ static inline void chash_dequeue_srv(struct server *s)
>   */
>  static inline void chash_queue_dequeue_srv(struct server *s)
>  {
> - while (s->lb_nodes_now > s->next_eweight) {
> - if (s->lb_nodes_now >= s->lb_nodes_tot) // should always be 
> false anyway
> - s->lb_nodes_now = s->lb_nodes_tot;
> - s->lb_nodes_now--;
> - if (s->proxy->lbprm.chash.last == 
> >lb_nodes[s->lb_nodes_now].node)
> - s->proxy->lbprm.chash.last = 
> chash_skip_node(s->lb_tree, s->proxy->lbprm.chash.last);
> - eb32_delete(>lb_nodes[s->lb_nodes_now].node);
> - }
> + unsigned int j;
> +
> + /* First we need to remove all of the server's entries from its tree
> +  * because a) the realloc will change all node pointers and b) the keys
> +  * may change.
> +  */
> + chash_dequeue_srv(s);

This part above. It completely removes the server from the tree before
re-adding it for any weight change. That's extremely costly. One case
that stresses this part is the slowstart mechanism. If you set 500
servers in a backend like this:

server-template s 0-499 127.0.0.1:8000 slowstart 20s weight 256

then you turn all of them down then up, in -master the process runs at
roughly 10% CPU for 20 seconds for the time it takes to progressively
add all nodes to the tree, while with the new version, the load grows
over 20 seconds till it reaches 100% and sometimes triggers the watchdog.

IMHO the full dequeue should only be done in the previous conditions,
or if the key changes, as you mentionned in the comment.

This means that you should store in the server along with the hash_key
the one which corresponds to the indexed nodes (the one that was last
used to insert the server in the tree). I'd just imagine a prev_hash_key
that's synchronized after inserting the server. Then when entering
chash_queue_dequeue_srv(), it would be sufficient to compare the two
keys and decide to call chash_dequeue_srv() in case they don't match.
This way the full dequeue would be done only when the key changes and
not when the weight slightly increases with a same key.

The only other comment I could find is this tiny one, in
chash_compute_node_key():

+   case SRV_HASH_KEY_ADDR_PORT:
+   server_get_inetaddr(s, _addr);
+   if (srv_addr.family != AF_INET && srv_addr.family != AF_INET6) {
+ hash_key = SRV_HASH_KEY_ID;
+   }

As you can see there's an indent issue before hash_key. I said it was
tiny ;-)

That's essentially all I could find, so I'm pretty confident that with
that addressed, the patch can be merged quickly!

Thank you!
Willy



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

2024-03-11 Thread Willy Tarreau
On Mon, Mar 11, 2024 at 11:24:34PM -0400, Anthony Deschamps wrote:
> Sorry for the trouble! I'll have to sort out what's happening.

No problem, some mailers are well-known for mangling what looks like
text.

> Here it is as an attachment.

Looks good as-is, thank you! I'll review it (hopefully today) and merge
it if I'm fine with it.

Willy



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

2024-03-11 Thread Willy Tarreau
Hi Anthony,

On Mon, Mar 11, 2024 at 08:58:17PM -0400, Anthony Deschamps wrote:
> > I'm not sure the scripts will help me (at least :-)). I was thinking that
> > a test could just be "set server XXX addr YYY" on the CLI to change the
> > server's address and verify that the hashing follows the address not the
> > server number.
> 
> Oh, good point -- I wasn't testing what happens when changing the address
> via the CLI. It turns out I wasn't handling that case. I added a call in
> _srv_set_inetaddr_port() to update the hashes, and tested that it works.
> 
> I also moved the docs and added an entry to the table.
> 
> Here's the updated patch.

Thanks!

However there's still a problem with your mailer mangling the patch
see below:

> + /* If hash-key is addr or addr-port then we need the address, but if we
> + * can't determine the address then we fall back on hashing the puid.
> + */
> + switch (s->hash_key) {
> + case SRV_HASH_KEY_ADDR:
> + case SRV_HASH_KEY_ADDR_PORT:
> + server_get_inetaddr(s, _addr);
> + if (srv_addr.family != AF_INET && srv_addr.family != AF_INET6) {
> +  hash_key = SRV_HASH_KEY_ID;
> + }
> + break;
(...)

Can you please resend it as an attachment instead ?

Thanks!
Willy



Re: Fix haproxy build on recent FreeBSD

2024-03-11 Thread Willy Tarreau
On Mon, Mar 11, 2024 at 05:56:35PM +, Brooks Davis wrote:
> > OK that works for me. Do you want to send a new patch or should I adapt
> > yours ? If you have a 12 somewhere that would save me time to verify I
> > don't mess up with the ifdefs, otherwise I can probably handle it and
> > we'll watch for any report of build error.
> 
> I don't have a 12 box handy either, but here's a patch that retains the
> old code wrapped in ifdefs and should be low risk as such.  1300058 is
> the next version bump after AT_EXECPATH support was added.

Much appreciated, thank you Brooks! I know someone with a 12 willing to
test (even though your patch should not break anything as-is).

I'm merging it now, thanks!
Willy



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

2024-03-09 Thread Willy Tarreau
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.

> If push comes to shove, one
> can always add an extra field somewhere, or an extra arg in get_addr_len,
> even if it'd be a little ugly.

Actually you're totally right and you make me realize that there's an
addrcmp field in the address families, and it's inconsistent to have
get_addr_len() being family-agnostic. So most likely we should first
change get_addr_len() to be per-family and appear in the proto_fam
struct. The few places where get_addr_len() is used should instead
rely on the protocol family for this. And due to this new address
having a variable length, we should support passing a pointer to the
address (like the current get_addr_len() does) in addition to the
protocol pointer.

Doing so would cleanly unlock all the problem. The new abns2 family
would just bring its own get_addr_len() variant that uses strlen().

This just shows how important it is to discuss about design ;-)

Cheers,
Willy



[ANNOUNCE] haproxy-3.0-dev5

2024-03-09 Thread Willy Tarreau
ample_to_logformat_list()
  CLEANUP: tree-wide: use proper ERR_* return values for PRE_CHECK fcts
  BUG/MINOR: cfgparse: report proper location for log-format-sd errors

Christopher Faulet (11):
  BUG/MEDIUM: applet: Fix HTX .rcv_buf callback function to release outbuf 
buffer
  BUG/MEDIUM: mux-h1: Fix again 0-copy forwarding of chunks with an unknown 
size
  BUG/MINOR: mux-h1: Properly report when mux is blocked during a nego
  MINOR: mux-h1: Move checks performed before a shutdown in a dedicated 
function
  MINOR: mux-h1: Move all stuff to detach a stream in an internal function
  MAJOR: mux-h1: Drain requests on client side before shut a stream down
  MEDIUM: htx/http-ana: No longer close connection on early HAProxy response
  CLEANUP: mux-h2: Fix h2s_make_data() comment about the return value
  BUG/MINOR: config/quic: Alert about PROXY protocol use on a QUIC listener
  BUG/MINOR: hlua: Fix log level to the right value when set via 
TXN:set_loglevel
  MINOR: hlua: Be able to disable logging from lua

Ilya Shipitsin (5):
  CLEANUP: assorted typo fixes in the code and comments
  CLEANUP: fix typo in naming for variable "unused"
  CI: run more smoke tests on config syntax to check memory related issues
  CI: enable monthly build only test on netbsd-9.3
  CI: skip scheduled builds on forks

Nenad Merdanovic (2):
  MINOR: vars: export var_set and var_unset functions
  MINOR: Add aes_gcm_enc converter

William Lallemand (4):
  BUG/MAJOR: ssl/ocsp: crash with ocsp when old process exit or using ocsp 
CLI
  BUG/MINOR: ssl/cli: duplicate cleaning code in cli_parse_del_crtlist
  DOC: configuration: clarify ciphersuites usage
  BUG/MINOR: ssl/cli: typo in new ssl crl-file CLI description

Willy Tarreau (12):
  BUG/MINOR: tools: seed the statistical PRNG slightly better
  BUG/MINOR: sink: fix a race condition in the TCP log forwarding code
  BUILD: thread: move lock label definitions to thread-t.h
  BUILD: tree-wide: fix a few missing includes in a few files
  BUILD: buf: make b_ncat() take a const for the source
  BUILD: ssl: define EVP_CTRL_AEAD_GET_TAG for older versions
  DOC: design: write first notes about ring-v2
  OPTIM: sink: try to merge "dropped" messages faster
  OPTIM: sink: drop the sink lock used to count drops
  DEV: haring: make haring not depend on the struct ring itself
  DEV: haring: split the code between ring and buffer
  DEV: haring: automatically use the advertised ring header size

matthias sweertvaegher (1):
  BUILD: solaris: fix compilation errors

---



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

2024-03-09 Thread Willy Tarreau
Hi Anthony,

On Wed, Feb 21, 2024 at 11:49:45AM -0500, Anthony Deschamps wrote:
> Hi Willy, thanks for the thoughtful feedback.
> 
> Here's a new patch that makes this configurable via a "hash-key" server
> argument, which defaults to "id" as you suggested.

Thanks.

> I'm struggling to test the last case you described. A quick description of
> my test setup: I have multiple instances of a simple echo server running
> on a range of ports, which I spawn/kill manually. I then have a local consul
> agent providing DNS, and multiple proxy instances running, and my actual
> test is a short Python script that makes a series of requests to each of the
> proxies and checks whether they were all routed to the same echo server
> (they identify themselves by setting a header in the response). I can share
> all the test scripts/config if it's helpful.

I'm not sure the scripts will help me (at least :-)). I was thinking that
a test could just be "set server XXX addr YYY" on the CLI to change the
server's address and verify that the hashing follows the address not the
server number.

> When I configure my proxy with long health checks -- by setting
>  "server-template ... check inter 6" -- I still find that when I
> kill/spawn an
> instance of the mock service, a server will first go down when an entry is
> removed from the SRV record, and then get rehashed when a new SRV
> entry is assigned to the server. Is there a different sequence of events I
> should be testing?

Well, if it does that and your server continues to get the traffic it
should get, then that's fine.

> Here is the updated patch.

Thank you. I'm seeing that your mailer mangled it (see below). Better
try again attaching it. Another detail, it looks like the doc entry is
located inside the "bind keywords" section, while it should be in section
4.1 "proxy keyword matrix", located between "hash-balance-factor" and
"hash-type". Please also make sure to add one entry in the keywords
table at the beginning of the section.

Oh and really do not hesitate to ping me again if I don't respond within
7 days, because it then becomes very hard for me to get back to a previous
discussion. I just remembered there was something to look for on the list,
but I could easily have lost it :-/

Thanks!
Willy

> >From 7d8a63803017c9b7630ec5cb3a154778fdff4d86 Mon Sep 17 00:00:00 2001
> From: Anthony Deschamps 
> Date: Fri, 16 Feb 2024 16:00:35 -0500
> Subject: [PATCH] MEDIUM: lb-chash: Deterministic node hashes based on server
>  address
> 
> Motivation: When services are discovered through DNS resolution, the order
> in
> which DNS records get resolved and assigned to servers is arbitrary.
> Therefore,
> even though two HAProxy instances using chash balancing might agree that a
> particular request should go to server3, it is likely the case that they
> have
> assigned different IPs and ports to the server in that slot.
> 
> This patch adds a server option, "hash-key " which can be set to "id"
> (the
> existing behaviour, defaut), "addr", or "addr-port". By deriving the keys
> for
> the chash tree nodes from a server's address and port we ensure that
> independent
> HAProxy instances will agree on routing decisions. If an address is not
> known
> then the key is derived from the server's puid as it was previously.
> 
> When adjusting a server's weight, we now unconditionally remove all nodes
> first,
> since the server's address (and therefore the desired node keys) may have
> changed.
> ---
>  doc/configuration.txt  | 21 
>  include/haproxy/server-t.h |  8 
>  src/lb_chash.c | 98 +-
>  src/server.c   | 28 +++
>  4 files changed, 132 insertions(+), 23 deletions(-)
> 
> diff --git a/doc/configuration.txt b/doc/configuration.txt
> index 1065e6098..a0710395a 100644
> --- a/doc/configuration.txt
> +++ b/doc/configuration.txt
> @@ -15763,6 +15763,27 @@ group 
>"gid" setting except that the group name is used instead of its gid. This
>setting is ignored by non UNIX sockets.
> 
> +hash-key 
> +  Specify how "hash-type consistent" node keys are computed
> +
> +  Arguments :
> +may be one of the following :
> +
> +  id The node keys will be derived from the server's position
> in
> + the server list.
> +
> +  addr   The node keys will be derived from the server's address,
> when
> + available, or else fall back on "id".
> +
> +  addr-port  The node keys will be derived from the server's address
> and
> + port, when available, or else fall back on "id".
> +
> +  The "addr" and "addr-port" options may be useful in scenarios where
> multiple
> +  HAProxy processes are balancing traffic to the same set of servers. If
> the
> +  server order of each process is different (because, for example, DNS
> records
> +  were resolved in different orders) then this will allow each independent
> +  HAProxy processes to agree on 

Re: [PATCH] MINOR: lb-chash: Respect maxconn when selecting a server

2024-03-09 Thread Willy Tarreau
Hi Anthony,

it seems I forgot about this thread, being sidetracked on other stuff...

On Wed, Feb 21, 2024 at 04:41:04PM -0500, Anthony Deschamps wrote:
> Hi Willy,
> 
> I wonder if I could accomplish what I'm looking to do by changing the
> behaviour of "maxqueue" (without making a breaking change, ideally).
> Currently, "maxqueue 0" is interpreted as "unlimited". However, the system
> I'm working with has long-running WebSocket connections, so a queued
> request is likely to sit in the queue for too long.
> 
> Is there a way to configure "maxqueue 0" for real? If not, then perhaps
> that's a patch I should make first, and then come back to this one. Since
> "maxqueue 0" already means "unlimited", maybe a "no-queue" option
> would be best in order to avoid making a breaking change.

Oh, sadly I didn't remember that maxqueue 0 was unlimited. Indeed, that's
annoying because I do see the value in totally preventing the queue from
being used. I guess we copied that from maxconn and other limits to
preserve the same logic of 0=unlimited. But that wasn't very wise for
this specific one :-/

Maybe (big maybe) we could change the default value to -1 for unlimited
as a few other settings have, and permit value zero to be used. The
problem I'm having with this is for APIs and those who automate their
config generation, and who might possibly already emit "maxqueue 0" to
make it infinite. In addition, the maxqueue feature was introduced 16
years ago in 1.3.14 by commit acafc5f88 ("[MEDIUM] add support for
"maxqueue" to limit server queue overload"), and documented a few
months later in 1.3.15 by commit 198a744e1 ("[DOC] all server parameters
have been documented"), so the risk is not that minimal.

I'm CCing Marko who develops the Dataplane API. This should be very
representative of how such values could be interpreted. Marko, do you
know if the dpapi ever emits "maxqueue 0" or makes a special case of
it when it finds it in a config ? Here the problem is that a poor
historical choice prevents one from saying that we don't want to queue
at all on a server.

I'd like to hear about others here on the list regarding this. If there's
a general consensus that it's mostly harmless to change the infinite from
0 to -1 and permit 0 to be used, we could do it and put it in the release
notes. Otherwise we'll need to find another solution (maybe an extra
keyword to disable any queuing).

Thanks!
willy



Re: [PR] BUILD: solaris: fix compilation errors

2024-03-09 Thread Willy Tarreau
On Tue, Feb 27, 2024 at 10:23:02AM +, PR Bot wrote:
> Dear list!
> 
> Author: matthias sweertvaegher <178714+mx...@users.noreply.github.com>
> Number of patches: 1
> 
> This is an automated relay of the Github pull request:
>BUILD: solaris: fix compilation errors

Now merged, thank you Matthias!

Willy



Re: Fix haproxy build on recent FreeBSD

2024-03-08 Thread Willy Tarreau
Hi Brooks,

On Fri, Mar 08, 2024 at 09:47:39PM +, Brooks Davis wrote:
> On Fri, Mar 08, 2024 at 06:19:42PM +0100, Willy Tarreau wrote:
> > Hi Dmitry,
> > 
> > first, sorry for the long delay but these days I've been drained in a
> > bunch of meetings and reviews that took more time than I expected!
> > 
> > On Wed, Feb 28, 2024 at 11:06:00PM +0300, Dmitry Sivachenko wrote:
> > > Hello!
> > > 
> > > Recently FreeBSD has moved some things out from libc to libsys (see e.g
> > > https://www.mail-archive.com/dev-commits-src-all@freebsd.org/msg50353.html)
> > > So haproxy stopped compiling with "ld: error: undefined symbol: 
> > > __elf_aux_vector" error.
> > > 
> > > Brooks Davis suggested the attached patch to fix that.
> > > 
> > > Please consider including it into the tree.
> > 
> > What's the oldest FreeBSD version that will build with this ? Shouldn't
> > we just guard it by version ? This patch is pretty well isolated and in
> > a place already full of ifdefs, so it would cost basically nothing to
> > add a test for the FreeBSD version in addition to defined():
> 
> It looks like __elf_aux_vector is available since FreeBSD 9,
> elf_aux_info since 12, and AT_EXECPATH support since 13.
> I belive it will build on 12, but elf_aux_info will return an error and
> leave execpath unmodified.  If you need to support earlier versions then
> I guess a __FreeBSD_version ifdef would be appropriate.

OK that works for me. Do you want to send a new patch or should I adapt
yours ? If you have a 12 somewhere that would save me time to verify I
don't mess up with the ifdefs, otherwise I can probably handle it and
we'll watch for any report of build error.

Thanks,
Willy



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

2024-03-08 Thread Willy Tarreau
On Fri, Mar 08, 2024 at 05:38:32PM +, Tristan wrote:
> Hi Willy,
> 
> On 08/03/2024 17:05, Willy Tarreau wrote:
> > We could just have "abns2" and declare that it's the second version of the
> > abns format and know that this one is interoperable with a number of other
> > components.
> 
> Having abns@ and abns2@ strictly for that one difference seems like a lot to
> me, to be honest, but I get the argument that they are otherwise
> fundamentally incompatible... Hmm...

Believe me, it's not much (in theory, we can always detect a long tail
and later decide to change our minds). I wouldn't recommend you a painful
option.

> > It would even be easier to deal with for those who document
> > such components because their integration docs would just indicate that
> > the address has to be placed after "abns2@" in haproxy, instead of saying
> > "and don't forget to adjust this global setting in your config, unless you
> > already use abns anywhere else".
> 
> Considering there's barely any documentation on the internet as a whole
> about abstract unix sockets, I doubt we should worry too much about that
> bit.
> Also the target audience for those is unlikely to be too deterred.

Maybe. One of the reasons might also be the failure from some to make
it interoperate with their tools, and they give up.

> > Indeed, right now it seems that your patch
> > does nothing for the receivers, bind() still uses sizeof(), and the
> > sock_unix_addrcmp() function continues to compare the full name, making
> > such sockets non-transferable across reloads. So it's unclear to me how
> > this can work in your test :-/
> 
> Maybe I'm missing some place/case, but at least for trivial binds it
> certainly works; which explains why the test passes :-)

Yes but based on the code I don't see how this is possible. bind() uses
sizeof() there, not get_addr_len().

> // without tightsocklen
> $ netstat -l | grep @haproxy
> unix  2  [ ACC ] STREAM LISTENING 934838 
> @haproxy-f1@
> 
> // with tightsocklen
> $ netstat -l | grep @haproxy
> unix  2  [ ACC ] STREAM LISTENING 956878   @haproxy-f1

OK thanks for the capture. I just fail to see what code this passes
through and that worries me a bit more.

> > Thus I think it would be preferable to go that route rather than having
> > to deal with all exceptions everywhere based on a global-only option.
> > We can help on this, of course.
> 
> Well I do want it so I'm fine with doing it. But I wonder if it's not a bit
> of a:
> - abns@ eventually not used by anyone as everything keeps on moving away
> from padding
> - abns2@ eventually the default, which is quite jarring I suppose

We don't care. It's like the proxy protocol, one is legacy and still quite
deployed, but all improvements come on the new version. If there's no cost
in keeping legacy stuff for those who already use it, let's keep it running
and don't worry about it. The only configs which will need to be adapted
are those which cannot interoperate with external tools anyway, so it's
not as if it would require more changes.

> If it's easier that way, I'd prefer making it a proper bind/server option,
> which allows for easier transition, and can just change default value down
> the line if necessary?

The problem with default values is that a single behavior cannot be deduced
from reading a single config statement. That's quite painful for lots of
people (including those who copy config blocks from stackoverflow), and
for API tools. And it works half of the time because internally both modes
work but they can't interoperate with other tools. Here we're really
indicating a new address format. There's nothing wrong with that and we
do have the tools to handle that.

I think that the plan should be:

  - add AF_CUST_ABNS2 to protocol-t.h before AF_CUST_MAX
  - add to str2sa_range() the case for "abns2" just after "abns" with
address family "AF_CUST_ABNS2", and duplicate the test for
ss.ss_family == AF_UNIX later for AF_CUST_ABNS2 removing all the
stuff related to !abstract, or instead, just extend the "if" condition
to the new family and add that case there as well (might be even easier).
  - duplicate "proto_fam_unix" to "proto_fam_abns2" in sock_unix.c, referencing
"sock_abns2_addrcmp" for the address comparison
  - duplicate sock_unix_addrcmp() to sock_abns2_addrcmp() and implement only
the abns case, basically:

if (a->ss_family != b->ss_family)
return -1;

if (a->ss_family != AF_CUST_ABNS2)
return -1;

if (au->sun_path[0])
  

Re: Fix haproxy build on recent FreeBSD

2024-03-08 Thread Willy Tarreau
Hi Dmitry,

first, sorry for the long delay but these days I've been drained in a
bunch of meetings and reviews that took more time than I expected!

On Wed, Feb 28, 2024 at 11:06:00PM +0300, Dmitry Sivachenko wrote:
> Hello!
> 
> Recently FreeBSD has moved some things out from libc to libsys (see e.g
> https://www.mail-archive.com/dev-commits-src-all@freebsd.org/msg50353.html)
> So haproxy stopped compiling with "ld: error: undefined symbol: 
> __elf_aux_vector" error.
> 
> Brooks Davis suggested the attached patch to fix that.
> 
> Please consider including it into the tree.

What's the oldest FreeBSD version that will build with this ? Shouldn't
we just guard it by version ? This patch is pretty well isolated and in
a place already full of ifdefs, so it would cost basically nothing to
add a test for the FreeBSD version in addition to defined():

> --- src/tools.c.orig
> +++ src/tools.c
> @@ -17,9 +17,7 @@
>  #endif
>  
>  #if defined(__FreeBSD__)
> -#include 
> -#include 
> -extern void *__elf_aux_vector;
> +#include 
>  #endif
>  
>  #if defined(__NetBSD__)
> @@ -5018,13 +5016,11 @@
>   if (execfn && execfn != ENOENT)
>   ret = (const char *)execfn;
>  #elif defined(__FreeBSD__)
> - Elf_Auxinfo *auxv;
> - for (auxv = __elf_aux_vector; auxv->a_type != AT_NULL; ++auxv) {
> - if (auxv->a_type == AT_EXECPATH) {
> - ret = (const char *)auxv->a_un.a_ptr;
> - break;
> - }
> - }
> + static char execpath[MAXPATHLEN];
> + if (execpath[0] == '\0')
> + elf_aux_info(AT_EXECPATH, execpath, MAXPATHLEN);
> + if (execpath[0] != '\0')
> + ret = execpath;
>  #elif defined(__NetBSD__)
>   AuxInfo *auxv;
>   for (auxv = _dlauxinfo(); auxv->a_type != AT_NULL; ++auxv) {

Note that I tested it on a 13.1 and it builds there, I'm just concerned
about older ones (e.g. I don't have a v12 here).

Thanks!
Willy



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

2024-03-08 Thread Willy Tarreau
Hi Tristan,

On Wed, Mar 06, 2024 at 07:32:55AM +, Tristan wrote:
> Hello,
> 
> Earlier, I ran into the issue outlined in
> https://github.com/haproxy/haproxy/issues/977
> 
> Namely, that HAProxy will NUL-pad (as suffix) abstract unix socket paths,
> causing interop issues with other programs.
> I raised a new feature request to make this behaviour tunable
> (https://github.com/haproxy/haproxy/issues/2479).

Yep, thanks for this.

> Since I do quite want this, I also came up with a patch for it, attached to
> this email.
> 
> Marked as RFC for now because there hasn't really been a broad discussion on
> the topic yet.
 
Sure. There's one thing that bothers me in the way the option is configured,
which is that "unix-bind" takes options from the "bind" line, and here we're
adding a special case where this option is only valid for this line, and it
experiences special treatment. If we do it the "unix-bind" way, it means the
option will also be valid for each "bind" line. I understand the problem it
could cause when parsing it (i.e. was the global setting already set or not)
but that also enlights one of the difficulties related to interpreting the
configuration based on settings in the global section.

I'm still tempted to think that this should be seen as a different address
family since that's basically what we're doing, changing the way addresses
are used. Indeed, except for socket names that are exactly 107-chars long,
there's no way any address of the previous family can interact with one of
the new family.

We could just have "abns2" and declare that it's the second version of the
abns format and know that this one is interoperable with a number of other
components. It would even be easier to deal with for those who document
such components because their integration docs would just indicate that
the address has to be placed after "abns2@" in haproxy, instead of saying
"and don't forget to adjust this global setting in your config, unless you
already use abns anywhere else".

I had a look, and I think everything begins in str2sa_range() in this
block:

else if (ss.ss_family == AF_UNIX) {
   ...
}

By adding a new AF_CUST_ABNS2 family I think we can extend sock_unix
to properly deal with that. Indeed, right now it seems that your patch
does nothing for the receivers, bind() still uses sizeof(), and the
sock_unix_addrcmp() function continues to compare the full name, making
such sockets non-transferable across reloads. So it's unclear to me how
this can work in your test :-/

Having an explicit family would allow to manage all of this at once:
mostly everywhere we have a test for AF_UNIX we could add || AF_CUST_ABNS2
and make the special case of the address length there. I don't *think*
the socket transfer mechanism in sock.c needs to be touched because
retrieving the socket's address should give us its original length.

Thus I think it would be preferable to go that route rather than having
to deal with all exceptions everywhere based on a global-only option.
We can help on this, of course.

Thanks!
Willy



Re: [PATCH 1/1] CI: skip scheduled builds on forks

2024-03-05 Thread Willy Tarreau
On Wed, Feb 21, 2024 at 05:05:39PM +0100, Ilya Shipitsin wrote:
> tracking bleeding edge changes with some rare platforms or modern
> compilers on scheduled basis is not what usually forks do. let's
> skip by default in forks, if some fork is interested, it might be
> enabled locally

Thank you! Indeed if we can save some resources it's always better.
I don't know the part of this job that currently works fine or fails
though. Now merged.

Willy



Re: [PATCH 1/1] CI: enable monthly build only test on netbsd-9.3

2024-03-05 Thread Willy Tarreau
On Mon, Feb 19, 2024 at 10:14:59PM +0100, Ilya Shipitsin wrote:
> it is interesting to try https://github.com/vmactions/netbsd-vm actions

Now merged, thanks!
Willy



Re: [PATCH 1/1] CI: run more smoke tests on config syntax to check memory related issues

2024-03-05 Thread Willy Tarreau
On Sat, Feb 17, 2024 at 08:42:28PM +0100, Ilya Shipitsin wrote:
> config syntax check seems add a value on testing code path not
> covered by VTest, also checks are very fast

Applied, thanks! We'll see if it triggers anything, that could
indeed be helpful.

Willy



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

2024-03-05 Thread Willy Tarreau
On Thu, Feb 22, 2024 at 10:12:27AM +0100, Ilya Shipitsin wrote:
> This is 39th iteration of typo fixes

Now merged, thank you! I split it in two because the one on
resolvers and stick-tables directly affects the code (it renames
a function argument) and I want to make it easy to drop it in case
backports need/don't need it.

Thanks,
Willy



Re: [PATCH 0/1] CI: additional ASAN smoke tests

2024-03-04 Thread Willy Tarreau
Hi Ilya,

On Mon, Mar 04, 2024 at 10:41:12PM +0100,  ??? wrote:
> ping :)

sorry, I wanted to double-check with others but forgot. Will do ASAP,
thanks!
Willy



[ANNOUNCE] haproxy-3.0-dev4

2024-02-23 Thread Willy Tarreau
lease
  BUG/MAJOR: promex: fix crash on deleted server
  BUG/MAJOR: server: fix stream crash due to deleted server
  BUG/MEDIUM: mux-quic: do not crash on qcs_destroy for connection error
  BUG/MINOR: quic: fix output of show quic

Aurelien DARRAGON (12):
  MINOR: log: custom name for logformat node
  MINOR: sample: add type_to_smp() helper function
  MINOR: log: explicit typecasting for logformat nodes
  MINOR: log: simplify last_isspace in sess_build_logline()
  MINOR: log: simplify quotes handling in sess_build_logline()
  MINOR: log: print metadata prefixes separately in sess_build_logline()
  MINOR: log: automate string array construction in sess_build_logline()
  CLEANUP: proxy/log: remove unused proxy flag
  CLEANUP: log: fix process_send_log() indentation
  CLEANUP: log: use free_logformat_list() in parse_logformat_string()
  MINOR: log: add free_logformat_node() helper function
  BUG/MINOR: log: fix potential lf->name memory leak

Christopher Faulet (22):
  BUG/MAJOR: mux-h1: Fix zero-copy forwarding when sending chunks of 
unknown size
  MINOR: stats: Use a dedicated function to check if output is almost full
  BUG/MEDIUM: applet: Add a flag to state an applet is using zero-copy 
forwarding
  BUG/MEDIUM: stconn/applet: Block 0-copy forwarding if producer needs more 
room
  MINOR: applet: Remove uselelss test on SE_FL_SHR/SHW flags
  MEDIUM: applet: Add notion of shutdown for write for applets
  MINOR: cli: No longer check SC for shutdown to interrupt wait command
  BUG/MEDIUM: stconn: Allow expiration update when READ/WRITE event is 
pending
  BUG/MEDIUM: stconn: Don't check pending shutdown to wake an applet up
  CLEANUP: stconn: Move SE flags set by app layer at the end of the bitfield
  MINOR: stconn: Rename SE_FL_MAY_FASTFWD and reorder bitfield
  MINOR: stconn: Add SE flag to announce zero-copy forwarding on consumer 
side
  MINOR: muxes: Announce support for zero-copy forwarding on consumer side
  BUG/MAJOR: stconn: Check support for zero-copy forwarding on both sides
  MINOR: muxes/applet: Simplify checks on options to disable zero-copy 
forwarding
  BUG/MEDIUM: applet: Immediately free appctx on early error
  BUG/MEDIUM: hlua: Be able to garbage collect uninitialized lua sockets
  BUG/MEDIUM: hlua: Don't loop if a lua socket does not consume received 
data
  BUG/MEDIUM: mux-h1: Don't emit 0-CRLF chunk in h1_done_ff() when iobuf is 
empty
  MINOR: cli: Remove useless loop on commands to find unescaped semi-colon
  BUG/MEDIUM: cli: Warn if pipelined commands are delimited by a \n
  BUG/MAJOR: cli: Restore non-interactive mode behavior with pipelined 
commands

Frederic Lecaille (5):
  BUG/MEDIUM: quic: Wrong K CUBIC calculation.
  MINOR: quic: Update K CUBIC calculation (RFC 9438)
  MINOR: quic: Dynamic packet reordering threshold
  MINOR: quic: Add a counter for reordered packets
  DOC: quic: Missing tuning setting in "Global parameters"

Miroslav Zagorac (1):
  MINOR: ssl: Call callback function after loading SSL CRL data

Nicolas CARPi (1):
  DOC/MINOR: userlists: mention solutions to high cpu with hashes

Remi Tricot-Le Breton (1):
  BUG/MEDIUM: ssl: Fix crash when calling "update ssl ocsp-response" when 
an update is ongoing

Willy Tarreau (2):
  BUILD: applet: fix build on some 32-bit archs
  BUG/MINOR: ist: only store NUL byte on succeeded alloc

---



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

2024-02-20 Thread Willy Tarreau
On Fri, Feb 16, 2024 at 05:03:56PM -0500, Anthony Deschamps wrote:
> >From a031cf97da759eb2c2f9b6e191065ad503f821ed Mon Sep 17 00:00:00 2001
> From: Anthony Deschamps 
> Date: Fri, 16 Feb 2024 16:00:35 -0500
> Subject: [PATCH] MEDIUM: lb-chash: Deterministic node hashes based on server
>  address
> 
> Motivation: When services are discovered through DNS resolution, the order
> in
> which DNS records get resolved and assigned to servers is arbitrary.
> Therefore,
> even though two HAProxy instances using chash balancing might agree that a
> particular request should go to server3, it is likely the case that they
> have
> assigned different IPs and ports to the server in that slot.
> 
> By deriving the keys for the chash tree nodes from a server's address and
> port
> we ensure that independent HAProxy instances will agree on routing
> decisions. If
> an address is not known then the key is derived from the server's puid as
> it was
> previously.
> 
> When adjusting a server's weight, we now unconditionally remove all nodes
> first,
> since the server's address (and therefore the desired node keys) may have
> changed.

That's a good idea, it corresponds more or less to what is already
supported for stick-tables where we can decide to synchronize the servers'
addresses instead of IDs.

However here you have replaced the existing method instead of offering
an option to choose this one, and that's not correct as it will break
a number of setups, for example those with multiple DCs and replicated
servers, or those with dual-attached servers (each connected to one LB),
or those who want to have all ports going to the same server, etc.

I think that your use case would be desirable for a majority of users,
so that's definitely something we should do, but making it configurable.

Since here the hash key is a property of the server, it would make sense
to add a "hash-key" argument to the server and support "id" (the default),
"addr" (IP only, useful when learned over DNS), "addr-port" for even more
mixing, and maybe in the future even "fqdn" or "name" depending on what
users ask for.

Another point to take care of is what happens when a server's address is
changed (DNS, CLI). Normally here you'll need to unhash / rehash the node.
In the current version it would become incorrect if the server doesn't
first go down then up (this can happen for example if health checks are
too far apart and give enough time for the server's address to be
reassigned).

Thanks!
Willy



Re: [PATCH] MINOR: lb-chash: Respect maxconn when selecting a server

2024-02-19 Thread Willy Tarreau
Hi Anthony,

On Tue, Feb 13, 2024 at 07:49:06PM -0500, Anthony Deschamps wrote:
> >From 3fc983b719bd4d8af80037c36e7032e0af383557 Mon Sep 17 00:00:00 2001
> From: Anthony Deschamps 
> Date: Tue, 13 Feb 2024 18:11:56 -0500
> Subject: [PATCH] MINOR: lb-chash: Respect maxconn when selecting a server
> 
> This is useful in a situation where hash-balance-factor isn't quite
> optimal. For example:
> 
> If we have two servers at times of low traffic, if server A has one active
> request and server B has
> zero, we might prefer that a second request for the same resource also be
> routed to server A; with
> hash-balance-factor set to anything less than 200, that request would
> routed to server B, causing it
> to dedicate resources to a resource that is already loaded/warm in server
> A.  Meanwhile, at times of
> high traffic, we still want multiple servers to be able to share the load
> of highly used resources.

I like the idea, however we shouldn't do it by default IMHO, because for
many users, maxconn is just a way to avoid bursts on the servers and almost
does not inflict perceptible delays. Here it would result in a significant
increase of the miss ratio for caches having a small maxconn that's often
reached and that's not always desirable.

I'm wondering if we shouldn't just add a new type for "hash-type". There
are already "map-based" and "consistent", maybe we should coin a new term
here ("bounded-consistent" or something shorter and more explanatory) ?

Alternately adding a new option in the backend would be possible as well,
of course. It would just require a bit of doc and explain that it only
applies to consistent hashing.

Thanks,
Willy



[ANNOUNCE] haproxy-2.8.6

2024-02-16 Thread Willy Tarreau
lle (11):
  BUG/MEDIUM: quic: keylog callback not called (USE_OPENSSL_COMPAT)
  CLEANUP: quic: Remove unused CUBIC_BETA_SCALE_FACTOR_SHIFT macro.
  MINOR: quic: Stop hardcoding a scale shifting value 
(CUBIC_BETA_SCALE_FACTOR_SHIFT)
  BUG/MINOR: quic: Wrong ack ranges handling when reaching the limit.
  CLEANUP: quic: Code clarifications for QUIC CUBIC (RFC 9438)
  BUG/MINOR: quic: fix possible integer wrap around in cubic window 
calculation
  MINOR: quic: Stop using 1024th of a second.
  BUG/MEDIUM: quic: Wrong K CUBIC calculation.
  MINOR: quic: Update K CUBIC calculation (RFC 9438)
  MINOR: quic: Dynamic packet reordering threshold
  MINOR: quic: Add a counter for reordered packets

Frédéric Lécaille (5):
  BUG/MEDIUM: quic: Possible buffer overflow when building TLS records
  BUG/MEDIUM: quic: QUIC CID removed from tree without locking
  BUG/MINOR: quic: Wrong keylog callback setting.
  BUG/MINOR: quic: Missing call to TLS message callbacks
  CLEANUP: quic: Remaining useless code into server part

Lukas Tribus (1):
  DOC: httpclient: add dedicated httpclient section

Miroslav Zagorac (1):
  DOC: configuration: corrected description of keyword 
tune.ssl.ocsp-update.mindelay

Olivier Houchard (1):
  BUG/MAJOR: ssl_sock: Always clear retry flags in read/write functions

Remi Tricot-Le Breton (10):
  BUG/MINOR: ssl: Fix error message after ssl_sock_load_ocsp call
  BUG/MINOR: ssl: Duplicate ocsp update mode when dup'ing ckch
  BUG/MINOR: ssl: Clear the ckch instance when deleting a crt-list line
  MINOR: ssl: Use OCSP_CERTID instead of ckch_store in 
ckch_store_build_certid
  BUG/MEDIUM: ocsp: Separate refcount per instance and per store
  BUG/MINOR: ssl: Destroy ckch instances before the store during deinit
  BUG/MINOR: ssl: Reenable ocsp auto-update after an "add ssl crt-list"
  REGTESTS: ssl: Fix empty line in cli command input
  REGTESTS: ssl: Add OCSP related tests
  BUG/MEDIUM: ssl: Fix crash when calling "update ssl ocsp-response" when 
an update is ongoing

Thayne McCombs (1):
  DOC: configuration: clarify http-request wait-for-body

Tim Duesterhus (1):
  CI: Update to actions/cache@v4

William Lallemand (4):
  DOC: configuration: typo req.ssl_hello_type
  BUG/MINOR: mworker/cli: fix set severity-output support
  BUG/MINOR: resolvers: default resolvers fails when network not configured
  MINOR: errors: ha_alert() and ha_warning() uses warn_exec_path()

Willy Tarreau (23):
  MINOR: mux-h2: support limiting the total number of H2 streams per 
connection
  BUG/MINOR: mux-h2: also count streams for refused ones
  MINOR: compiler: add a new DO_NOT_FOLD() macro to prevent code folding
  MINOR: debug: make sure calls to ha_crash_now() are never merged
  MINOR: debug: make ABORT_NOW() store the caller's line number when using 
abort
  MINOR: debug: make BUG_ON() catch build errors even without DEBUG_STRICT
  MINOR: mux-h2/traces: also suggest invalid header upon parsing error
  MINOR: mux-h2/traces: explicitly show the error/refused stream states
  MINOR: mux-h2/traces: clarify the "rejected H2 request" event
  BUG/MEDIUM: mux-h2: refine connection vs stream error on headers
  MINOR: mux-h2/traces: add a missing trace on connection WU with negative 
inc
  BUG/MINOR: vars/cli: fix missing LF after "get var" output
  BUG/MEDIUM: cli: fix once for all the problem of missing trailing LFs
  BUG/MINOR: jwt: fix jwt_verify crash on 32-bit archs
  BUG/MEDIUM: pool: fix rare risk of deadlock in pool_flush()
  BUG/MINOR: h1-htx: properly initialize the err_pos field
  BUG/MEDIUM: h1: always reject the NUL character in header values
  BUG/MINOR: diag: always show the version before dumping a diag warning
  BUG/MINOR: diag: run the final diags before quitting when using -c
  MINOR: ext-check: add an option to preserve environment variables
  BUILD: address a few remaining calloc(size, n) cases
  DOC: internal: update missing data types in peers-v2.0.txt
  DEV: makefile: add a new "range" target to iteratively build all commits

---



Re: [PATCH] DOC/MINOR: userlists: musl performance

2024-02-16 Thread Willy Tarreau
On Mon, Feb 12, 2024 at 06:42:25PM +0100, Lukas Tribus wrote:
> On Mon, 12 Feb 2024 at 18:10, Nicolas CARPi  wrote:
> >
> > Dear Lukas, Willy,
> >
> > Please find another patch attached, addressing your comments.
> >
> > Willy: s/gcc/glibc/
> >
> > Lukas: I shifted the focus on the rounds/cost solution, while still
> > mentioning the musl issue, as this problem is clearly more visible on
> > Alpine Linux, as the github issues show.
> 
> Thank you, I agree.
> 
> Acked-by: Lukas Tribus 

Thanks to you both. Unfortunately I missed it before the 2.9/2.8 releases,
but I'll merge it anyway.

Willy



[ANNOUNCE] haproxy-2.9.5

2024-02-15 Thread Willy Tarreau
eviewed-bugs
   Code reports : https://www.haproxy.org/l/code-reports
   Latest builds: https://www.haproxy.org/l/dev-packages

Willy
---
Complete changelog :
Abhijeet Rastogi (1):
  DOC: install: recommend pcre2

Aurelien DARRAGON (4):
  BUILD: debug: remove leftover parentheses in ABORT_NOW()
  DOC: config: fix misplaced "txn.conn_retries"
  DOC: config: fix typos for "bytes_{in,out}"
  DOC: config: fix misplaced "bytes_{in,out}"

Christopher Faulet (12):
  BUG/MEDIUM: stconn: Allow expiration update when READ/WRITE event is 
pending
  BUG/MEDIUM: stconn: Don't check pending shutdown to wake an applet up
  CLEANUP: stconn: Move SE flags set by app layer at the end of the bitfield
  MINOR: stconn: Rename SE_FL_MAY_FASTFWD and reorder bitfield
  MINOR: stconn: Add SE flag to announce zero-copy forwarding on consumer 
side
  MINOR: muxes: Announce support for zero-copy forwarding on consumer side
  BUG/MAJOR: stconn: Check support for zero-copy forwarding on both sides
  MINOR: muxes/applet: Simplify checks on options to disable zero-copy 
forwarding
  BUG/MEDIUM: mux-h2: Switch pending error to error if demux buffer is empty
  BUG/MEDIUM: mux-h2: Only Report H2C error on read error if demux buffer 
is empty
  BUG/MEDIUM: mux-h2: Don't report error on SE if error is only pending on 
H2C
  BUG/MEDIUM: mux-h2: Don't report error on SE for closed H2 streams

Frederic Lecaille (9):
  BUG/MINOR: quic: Wrong ack ranges handling when reaching the limit.
  BUILD: quic: Variable name typo inside a BUG_ON().
  CLEANUP: quic: Code clarifications for QUIC CUBIC (RFC 9438)
  BUG/MINOR: quic: fix possible integer wrap around in cubic window 
calculation
  MINOR: quic: Stop using 1024th of a second.
  BUG/MEDIUM: quic: Wrong K CUBIC calculation.
  MINOR: quic: Update K CUBIC calculation (RFC 9438)
  MINOR: quic: Dynamic packet reordering threshold
  MINOR: quic: Add a counter for reordered packets

Remi Tricot-Le Breton (10):
  BUG/MINOR: ssl: Fix error message after ssl_sock_load_ocsp call
  BUG/MINOR: ssl: Duplicate ocsp update mode when dup'ing ckch
  BUG/MINOR: ssl: Clear the ckch instance when deleting a crt-list line
  MINOR: ssl: Use OCSP_CERTID instead of ckch_store in 
ckch_store_build_certid
  BUG/MEDIUM: ocsp: Separate refcount per instance and per store
  BUG/MINOR: ssl: Destroy ckch instances before the store during deinit
  BUG/MINOR: ssl: Reenable ocsp auto-update after an "add ssl crt-list"
  REGTESTS: ssl: Fix empty line in cli command input
  REGTESTS: ssl: Add OCSP related tests
  BUG/MEDIUM: ssl: Fix crash when calling "update ssl ocsp-response" when 
an update is ongoing

Tim Duesterhus (1):
  CI: Update to actions/cache@v4

Willy Tarreau (11):
  BUG/MINOR: diag: always show the version before dumping a diag warning
  BUG/MINOR: diag: run the final diags before quitting when using -c
  MINOR: compiler: add a new DO_NOT_FOLD() macro to prevent code folding
  MINOR: debug: make sure calls to ha_crash_now() are never merged
  MINOR: debug: make ABORT_NOW() store the caller's line number when using 
abort
  MINOR: debug: make BUG_ON() catch build errors even without DEBUG_STRICT
  DOC: internal: update missing data types in peers-v2.0.txt
  BUG/MINOR: vars/cli: fix missing LF after "get var" output
  BUG/MEDIUM: cli: fix once for all the problem of missing trailing LFs
  BUILD: address a few remaining calloc(size, n) cases
  BUG/MEDIUM: pool: fix rare risk of deadlock in pool_flush()

---



Re: [PATCH] DOC/MINOR: userlists: musl performance

2024-02-12 Thread Willy Tarreau
Hi Nicolas,

On Mon, Feb 12, 2024 at 02:13:18PM +0100, Nicolas CARPi wrote:
> Hello everyone,
> 
> Please find attached my very first patch to the documentation. Hope I 
> did everything good! :)

Thank you! I have one comment below:

> +  Furthermore, there is a significant performance penalty when using a musl
> +  variant (for instance with the Alpine linux distribution), as the musl 
> code for
> +  these hashing functions is less performant/optimized than its GCC
   ~~~
I guess you meant "glibc" instead of GCC above ?  :-)
If that's it, no worries, I can fix it and commit your patch so that
you don't need to resend it, otherwise feel free to respin another
version or to suggest an edit.

Thanks!
Willy



[ANNOUNCE] haproxy-3.0-dev3

2024-02-10 Thread Willy Tarreau
n't interrupt processing on partial post
  MAJOR: stats: Update HTTP stats applet to handle its own buffers
  MEDIUM: cache: Temporarily remove zero-copy forwarding support
  MAJOR: cache: Update HTTP cache applet to handle its own buffers
  MAJOR: cache: Send cached objects using zero-copy forwarding
  MINOR: stconn: Add support for flags during zero-copy forwarding 
negotiation
  MINOR: mux-h1: Be able to define the length of a chunk size when it is 
prepended
  MEDIUM: stconn: Nofify requested size during zero-copy forwarding nego is 
exact
  MINOR: mux-h1: Stop zero-copy forwarding during nego for too big 
requested size
  MEDIUM: mux-h1: Support zero-copy forwarding for chunks with an unknown 
size
  MAJOR: stats: Send stats dump over HTTP using zero-copy forwarding
  MEDIUM: applet: Simplify a bit API to exchange data with applets
  MINOR: cache: Remove unsed .data_sent field from the cache applet context
  MINOR: applet: Use an option to disable zero-copy forwarding for all 
applets
  MINOR: applet: Identify applets using their own buffers via a flag
  BUG/MINOR: applet: Always release empty appctx buffers after processing

Frederic Lecaille (5):
  BUG/MINOR: quic: Wrong ack ranges handling when reaching the limit.
  BUILD: quic: Variable name typo inside a BUG_ON().
  CLEANUP: quic: Code clarifications for QUIC CUBIC (RFC 9438)
  BUG/MINOR: quic: fix possible integer wrap around in cubic window 
calculation
  MINOR: quic: Stop using 1024th of a second.

Ilya Shipitsin (2):
  CI: github: abandon asan matrix.py helper
  CI: ssl: add yet another OpenSSL download fallback

Lukas Tribus (2):
  DOC: httpclient: add dedicated httpclient section
  DOC: install: clarify WolfSSL chroot requirements

Miroslav Zagorac (1):
  CLEANUP: log: deinitialization of the log buffer in one function

Olivier Houchard (1):
  BUG/MAJOR: ssl_sock: Always clear retry flags in read/write functions

Remi Tricot-Le Breton (9):
  BUG/MINOR: ssl: Fix error message after ssl_sock_load_ocsp call
  BUG/MINOR: ssl: Duplicate ocsp update mode when dup'ing ckch
  MINOR: ssl: Use OCSP_CERTID instead of ckch_store in 
ckch_store_build_certid
  BUG/MINOR: ssl: Clear the ckch instance when deleting a crt-list line
  BUG/MEDIUM: ocsp: Separate refcount per instance and per store
  BUG/MINOR: ssl: Destroy ckch instances before the store during deinit
  BUG/MINOR: ssl: Reenable ocsp auto-update after an "add ssl crt-list"
  REGTESTS: ssl: Add OCSP related tests
  REGTESTS: ssl: Fix empty line in cli command input

Thayne McCombs (1):
  DOC: configuration: clarify http-request wait-for-body

Tim Duesterhus (1):
  CI: Update to actions/cache@v4

William Lallemand (3):
  MINOR: ssl: add HAVE_SSL_0RTT constant
  MINOR: ssl: rename HA_OPENSSL_HAVE_0RTT_SUPPORT constant to 
HAVE_SSL_0RTT_QUIC
  MEDIUM: ssl/quic: always compile the ssl_conf.early_data test

Willy Tarreau (33):
  BUG/MINOR: h1-htx: properly initialize the err_pos field
  BUG/MEDIUM: h1: always reject the NUL character in header values
  CLEANUP: h1: remove unused function h1_measure_trailers()
  MINOR: compiler: add a new DO_NOT_FOLD() macro to prevent code folding
  MINOR: debug: make sure calls to ha_crash_now() are never merged
  MINOR: debug: make ABORT_NOW() store the caller's line number when using 
abort
  BUG/MINOR: diag: always show the version before dumping a diag warning
  BUG/MINOR: diag: run the final diags before quitting when using -c
  MINOR: acl: add extra diagnostics about suspicious string patterns
  MINOR: debug: make BUG_ON() catch build errors even without DEBUG_STRICT
  MINOR: debug: support passing an optional message in ABORT_NOW()
  MINOR: debug: add an optional message argument to the BUG_ON() family
  DEBUG: make the "debug dev {debug|warn|check}" command print a message
  BUG/MINOR: mux-h2: count rejected DATA frames against the connection's 
flow control
  MINOR: mux-h2: count excess of CONTINUATION frames as a glitch
  MINOR: mux-h2: count late reduction of INITIAL_WINDOW_SIZE as a glitch
  DOC: internal: update missing data types in peers-v2.0.txt
  MEDIUM: stick-tables: add a new stored type for glitch_cnt and glitch_rate
  MINOR: session: add the necessary functions to update the per-session 
glitches
  MEDIUM: mux-h2: update session trackers with number of glitches
  BUG/MINOR: server/cli: add missing LF at the end of certain notice/error 
lines
  BUG/MINOR: vars/cli: fix missing LF after "get var" output
  BUG/MEDIUM: cli: fix once for all the problem of missing trailing LFs
  MINOR: cli: make sure to always print a pending message after release()
  MINOR: cli: always reset the applet task's timeout
  MINOR: cli: add a new "wait" command to wait for a ce

Re: [PATCH] CI: Update to actions/cache@v4

2024-02-09 Thread Willy Tarreau
Hi Tim!

On Thu, Feb 08, 2024 at 07:55:23PM +0100, Tim Duesterhus wrote:
> No functional change, but this upgrade is required, due to the v3 runtime 
> being
> deprecated:
> 
> > Node.js 16 actions are deprecated. Please update the following actions to 
> > use
> > Node.js 20: actions/cache@v3. For more information see:
> > https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/.

I indeed noticed them recently. Now applied, thank you!
Willy



Re: pcre vs pcre2, which one to use?

2024-02-07 Thread Willy Tarreau
On Wed, Feb 07, 2024 at 07:07:13PM -0800, Abhijeet Rastogi wrote:
> Hi Willy,
> 
> Thanks for the quick clarification. I've sent a patch.
> 
> I also changed the "Quick build & install" section in the INSTALL doc to
> use USE_PCRE2, so folks don't accidently use the older version. I hope that
> was an intended change.

That's perfect, thank you very much, now applied!

Willy



  1   2   3   4   5   6   7   8   9   10   >