Re: [haproxy/haproxy] OPTIM/MINOR: h2_settings_initial_window_size default 64k (PR #1732)

2022-06-08 Thread Willy Tarreau
On Wed, Jun 08, 2022 at 09:22:31AM -0400, Glenn Strauss wrote:
> Since DATA frames might be in flight on the network, the server may want
> to be able to buffer twice the advertisted window size and defer sending
> WINDOW_UPDATE once the advertised window size is buffered.  Doing so
> gives the elephant in the pipe just enough space for a well-behaved
> client adhering to the window size restrictions, and without the
> undesirable thought of delaying the send WINDOW_UPDATE frames by RTT.

Yes exactly. I wasn't sure if you had taken into consideration that it
required extra buffering, which was the point of me raising that warning.
But it looks like you're handling this so it looks OK from my POV as
well.

Willy



Re: Ability to set response in lua via HTTP class

2022-06-08 Thread Yuriy Ivkin

Greetings!

 Is the TXN.done(txn[, reply]) function in haproxy 2.6 what I am 
looking for ?


08.06.2022 12:53, Yuriy Ivkin пишет:


Greetings!

 In the lua HTTP class did not find a method allows to override 
response body. May be it is possible via some other methods ?


 If not, are there any reasons should prevent me to write it ?

--
Best regards,
Yuriy Ivkin


--
Best regards,
Yuriy Ivkin


Re: [haproxy/haproxy] OPTIM/MINOR: h2_settings_initial_window_size default 64k (PR #1732)

2022-06-08 Thread Glenn Strauss
On Wed, Jun 08, 2022 at 02:42:10PM +0200, Willy Tarreau wrote:
> On Wed, Jun 08, 2022 at 08:29:48AM -0400, Glenn Strauss wrote:
> > > > Another approach might be to pre-emptively effectively double the
> > > > window size by sending WINDOW_UPDATE with the entire window size (65535)
> > > > back to the client upon receive of first DATA frame, and then keeping
> > > > track on server-side until that 65535 is exhausted before sending
> > > > another WINDOW_UPDATE.
> > > 
> > > Sending too large stream window sizes is problematic when some streams
> > > can be processed slowly, as it easily causes head-of-line blocking. If
> > > the advertised window is larger than the available storage, a paused
> > > stream will block other ones.
> > 
> >  we know that poorly written code knows no bounds.  Since the
> > server is permitted to increase the window size, inability by the client
> > to increase the window size is a bug in the client.  The client need
> > only track the window size -- the client need not actually use the extra
> > window size if the client does not wish to do so.
> 
> I was not speaking about the client but the server. If the server
> advertises more than it can buffer itself, you can end up with blocked
> streams: there's an elephant in the pipe that you can't pull completely
> because you have nowhere to store it, and because of this you can't
> access other frames that follow (the funniest ones being window updates
> from the client that are expected to release some room on the response
> side, but that basically only happens with echo servers). But you can
> leave headers frames hanging making the client think the server is taking
> time to process the requests while in fact the requests are not yet parsed,
> they're just blocked.

The generic statement you make is true, but does not apply here unless
the server would not have sent the WINDOW_UPDATE to replenish the
window, in which case the answer is to defer sending the WINDOW_UPDATE.

Since DATA frames might be in flight on the network, the server may want
to be able to buffer twice the advertisted window size and defer sending
WINDOW_UPDATE once the advertised window size is buffered.  Doing so
gives the elephant in the pipe just enough space for a well-behaved
client adhering to the window size restrictions, and without the
undesirable thought of delaying the send WINDOW_UPDATE frames by RTT.

Cheers, Glenn



Re: grooming IUS haproxy packages

2022-06-08 Thread William Lallemand
On Wed, Jun 01, 2022 at 07:17:33PM +0500, Илья Шипицин wrote:
> spec files work under centos 8 as well, but IUS currently builds only
> centos 7, I haven't figured out how to add centous 8 yet
> 

IUS seems to only support CentOS 7 unfortunately. :(

We had a talk with the guys from zenetys.com which are haproxy users for
a long time and they told us that they are maintaining packages for RHEL
6/7/8.

https://packages.zenetys.com/latest/redhat/
https://github.com/zenetys/rpm-haproxy

I added the links on https://github.com/haproxy/wiki/wiki/Packages .

-- 
William Lallemand



Re: [haproxy/haproxy] OPTIM/MINOR: h2_settings_initial_window_size default 64k (PR #1732)

2022-06-08 Thread Willy Tarreau
On Wed, Jun 08, 2022 at 08:29:48AM -0400, Glenn Strauss wrote:
> > I agree that it's independent but it's the one that is not expected to
> > cause any regression with any possible client. That's why I'd like to
> > have the two. First that one because it should be durable. Second, your
> > patch as an optimization, and if anyone complains about trouble with it
> > we can safely revert it or decide what to do without worrying about all
> > deployed curl clients that will not be updated.
> 
> Were this to cause a theoretical regression, this default initial window
> size can be configured in haproxy, so the workaround is to configure the
> value back to 65535.

Sure, that can be a workaround. But when it causes regressions in the
middle of a stable branch and affects users, the correct fix is to revert
and keep it only for major releases, with release notes advertising the
change of behavior.

> > > The upcoming lighttpd 1.4.65
> > > does just that, collecting updates until 16384 is reached, and then
> > > sending a WINDOW_UPDATE for 16384.  Cost: a single 1-RTT delay in
> > > sending a WINDOW_UPDATE to continue uploads larger than 112k-1.
> > 
> > It can definitely cause some inefficient clients to hang if they need
> > their buffer to be fully acked and their size is not a multiple of the
> > frame size. Be careful about this.
> 
> I ended up adjusting this before releasing lighttpd 1.4.65.
> 
> For the same few instructions, lighttpd 1.4.65 now sends WINDOW_UPDATE
> for 16384 when lighttpd receives the first DATA frame containing data
> (1-16384 bytes), and then does not send WINDOW_UPDATE until the next
> 16384 block is started.  This avoids any delay in sending WINDOW_UPDATE,
> though it does (temporarily) increase the window size allowed in the
> client if the client has not already sent data consuming the window.

OK.

> > > Another approach might be to pre-emptively effectively double the
> > > window size by sending WINDOW_UPDATE with the entire window size (65535)
> > > back to the client upon receive of first DATA frame, and then keeping
> > > track on server-side until that 65535 is exhausted before sending
> > > another WINDOW_UPDATE.
> > 
> > Sending too large stream window sizes is problematic when some streams
> > can be processed slowly, as it easily causes head-of-line blocking. If
> > the advertised window is larger than the available storage, a paused
> > stream will block other ones.
> 
>  we know that poorly written code knows no bounds.  Since the
> server is permitted to increase the window size, inability by the client
> to increase the window size is a bug in the client.  The client need
> only track the window size -- the client need not actually use the extra
> window size if the client does not wish to do so.

I was not speaking about the client but the server. If the server
advertises more than it can buffer itself, you can end up with blocked
streams: there's an elephant in the pipe that you can't pull completely
because you have nowhere to store it, and because of this you can't
access other frames that follow (the funniest ones being window updates
from the client that are expected to release some room on the response
side, but that basically only happens with echo servers). But you can
leave headers frames hanging making the client think the server is taking
time to process the requests while in fact the requests are not yet parsed,
they're just blocked.

Willy



Re: [haproxy/haproxy] OPTIM/MINOR: h2_settings_initial_window_size default 64k (PR #1732)

2022-06-08 Thread Glenn Strauss
On Wed, Jun 08, 2022 at 08:28:18AM +0200, Willy Tarreau wrote:
> On Tue, Jun 07, 2022 at 05:24:09PM -0400, Glenn Strauss wrote:
> > Yes, having the server merge the sizes of small DATA frames into a
> > larger WINDOW_UPDATE might be useful, too, but is independent and
> > complimentary to my patch proposed in
> > https://github.com/haproxy/haproxy/pull/1732

*complementary  [correcting myself]

> I agree that it's independent but it's the one that is not expected to
> cause any regression with any possible client. That's why I'd like to
> have the two. First that one because it should be durable. Second, your
> patch as an optimization, and if anyone complains about trouble with it
> we can safely revert it or decide what to do without worrying about all
> deployed curl clients that will not be updated.

Were this to cause a theoretical regression, this default initial window
size can be configured in haproxy, so the workaround is to configure the
value back to 65535.

> > One approach might be to collect small DATA frames and send a
> > WINDOW_UPDATE once a threshold is reached.
> 
> I thought about that one, focusing on 1/16 of a max frame size at once,
> but I don't like it much because it means we'd delay some WU, and the
> H2 spec was very clear about the fact that we ought to do our best to
> timely ack data in order to avoid deadlocks. You can have clients that
> stop sending until they're able to read a new block of a preconfigured
> window size for example (and that's where 65535 was an absurd choice
> in the first place). For example, a client that is hard-coded to use
> a 65535 buffer and needs to wait for the WU to replace that buffer
> would not receive an update for the block that spans from 61440 to
> 65535 and would not update the buffer. Thus I really want to be certain
> that we do not let processed DATA frames without the equivalent WU sent
> back.
> 
> > The upcoming lighttpd 1.4.65
> > does just that, collecting updates until 16384 is reached, and then
> > sending a WINDOW_UPDATE for 16384.  Cost: a single 1-RTT delay in
> > sending a WINDOW_UPDATE to continue uploads larger than 112k-1.
> 
> It can definitely cause some inefficient clients to hang if they need
> their buffer to be fully acked and their size is not a multiple of the
> frame size. Be careful about this.

I ended up adjusting this before releasing lighttpd 1.4.65.

For the same few instructions, lighttpd 1.4.65 now sends WINDOW_UPDATE
for 16384 when lighttpd receives the first DATA frame containing data
(1-16384 bytes), and then does not send WINDOW_UPDATE until the next
16384 block is started.  This avoids any delay in sending WINDOW_UPDATE,
though it does (temporarily) increase the window size allowed in the
client if the client has not already sent data consuming the window.

> > Another approach might be to pre-emptively effectively double the
> > window size by sending WINDOW_UPDATE with the entire window size (65535)
> > back to the client upon receive of first DATA frame, and then keeping
> > track on server-side until that 65535 is exhausted before sending
> > another WINDOW_UPDATE.
> 
> Sending too large stream window sizes is problematic when some streams
> can be processed slowly, as it easily causes head-of-line blocking. If
> the advertised window is larger than the available storage, a paused
> stream will block other ones.

 we know that poorly written code knows no bounds.  Since the
server is permitted to increase the window size, inability by the client
to increase the window size is a bug in the client.  The client need
only track the window size -- the client need not actually use the extra
window size if the client does not wish to do so.

Cheers, Glenn



Ability to set response in lua via HTTP class

2022-06-08 Thread Yuriy Ivkin

Greetings!

 In the lua HTTP class did not find a method allows to override 
response body. May be it is possible via some other methods ?


 If not, are there any reasons should prevent me to write it ?

--
Best regards,
Yuriy Ivkin


Source IP in Status WEBpage

2022-06-08 Thread Henning Svane
Hi

In PFsense implementation of HAProxy it is possible to see who, with there IP 
number, are sending traffic through the loadbalancer.

How can I do the same. I have look at there autogenerated configuration, but 
cannot get the same to work under HAproxy under Ubunutu.
I am using
HAProxy version 2.5.7-1ppa1~focal 2022/05/14

Regards
Henning


[PATCH] MINOR : converter: add param converter

2022-06-08 Thread astrothayne
From: Thayne McCombs 

Add a converter that extracts a parameter from string of delimited
key/value pairs.

Fixes: #1697
---
 doc/configuration.txt | 26 
 reg-tests/converter/param.vtc | 80 +++
 src/sample.c  | 64 ++--
 3 files changed, 167 insertions(+), 3 deletions(-)
 create mode 100644 reg-tests/converter/param.vtc

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 927c97ce3..bce29ef48 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -17411,6 +17411,32 @@ or()
   This prefix is followed by a name. The separator is a '.'. The name may only
   contain characters 'a-z', 'A-Z', '0-9', '.' and '_'.
 
+param(,[])
+  This extracts the first occurrence of the parameter  in the input 
string
+  where parameters are delimited by , which defaults to "&", and the 
name
+  and value of the parameter are separated by a "=". If there is no "=" and 
value
+  before the end of the parameter segment, it is treated as equivalent to a 
value
+  of an empty string.
+
+  This can be useful for extracting parameters from a query string, or 
possibly a
+  x-www-form-urlencoded body. In particular, `query,param()` can be used 
as
+  an alternative to `urlp()` which only uses "&" as a delimiter, whereas 
"urlp"
+  also uses "?" and ";".
+
+  Note that this converter doesn't do anything special with url encoded 
characters. If
+  you want to decode the value, you can use the url_dec converter on the 
output. If
+  the name of the parameter in the input might contain encoded characters, 
you'll probably
+  want do normalize the input before calling "param". This can be done using
+  "http-request normalize-uri", in particular the percent-decode-unreserved and
+  percent-to-uppercase options.
+
+  Example :
+  str(a=b=d=r),param(a)   # b
+  str(a=c),param(a) # ""
+  str(a==a),param(b)  # ""
+  str(a=1;b=2;c=4),param(b,;) # 2
+  query,param(redirect_uri),urldec()
+
 protobuf(,[])
   This extracts the protocol buffers message field in raw mode of an input 
binary
   sample representation of a protocol buffer message with  as 
field
diff --git a/reg-tests/converter/param.vtc b/reg-tests/converter/param.vtc
new file mode 100644
index 0..163360382
--- /dev/null
+++ b/reg-tests/converter/param.vtc
@@ -0,0 +1,80 @@
+varnishtest "param converter Test"
+
+feature ignore_unknown_macro
+
+server s1 {
+   rxreq
+   txresp -hdr "Connection: close"
+} -repeat 10 -start
+
+haproxy h1 -conf {
+   defaults
+   mode http
+   timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+   timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+   timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+
+   frontend fe
+   bind "fd@${fe}"
+
+   ### requests
+   http-request set-var(txn.query) query
+   http-response set-header Found %[var(txn.query),param(test)] if { 
var(txn.query),param(test) -m found }
+
+   default_backend be
+
+   backend be
+   server s1 ${s1_addr}:${s1_port}
+} -start
+
+client c1 -connect ${h1_fe_sock} {
+   txreq -url "/foo/?test=1=4"
+   rxresp
+   expect resp.status == 200
+   expect resp.http.found == "1"
+
+   txreq -url "/?a=1=4=34"
+   rxresp
+   expect resp.status == 200
+   expect resp.http.found == "34"
+
+   txreq -url "/?test=bar"
+   rxresp
+   expect resp.status == 200
+   expect resp.http.found == "bar"
+
+   txreq -url "/?a=b=d"
+   rxresp
+   expect resp.status == 200
+   expect resp.http.found == ""
+
+   txreq -url "/?a=b=t=d"
+   rxresp
+   expect resp.status == 200
+   expect resp.http.found == "t"
+
+   txreq -url "/?a=b=d"
+   rxresp
+   expect resp.status == 200
+   expect resp.http.found == ""
+
+   txreq -url "/?test="
+   rxresp
+   expect resp.status == 200
+   expect resp.http.found == ""
+
+txreq -url "/?a=b"
+rxresp
+expect resp.status == 200
+expect resp.http.found == ""
+
+txreq -url "/?testing=123"
+rxresp
+expect resp.status == 200
+expect resp.http.found == ""
+
+txreq -url "/?testing=123=4"
+rxresp
+expect resp.status == 200
+expect resp.http.found == "4"
+} -run
diff --git a/src/sample.c b/src/sample.c
index 237b88056..b2c80b6c8 100644
--- a/src/sample.c
+++ b/src/sample.c
@@ -2582,6 +2582,65 @@ static int sample_conv_word(const struct arg *arg_p, 
struct sample *smp, void *p
return 1;
 }
 
+static int sample_conv_param_check(struct arg *arg, struct sample_conv *conv,
+   const char *file, int line, char **err)
+{
+   if (arg[1].type == ARGT_STR && arg[1].data.str.data != 1) {
+   memprintf(err, "Delimiter must be exactly 1 character.");
+   return 0;
+   }
+
+   return 1;
+}
+
+static int sample_conv_param(const struct arg *arg_p, struct sample *smp, void 
*private)
+{

Re: [haproxy/haproxy] OPTIM/MINOR: h2_settings_initial_window_size default 64k (PR #1732)

2022-06-08 Thread Willy Tarreau
Hello Glenn,

On Tue, Jun 07, 2022 at 05:24:09PM -0400, Glenn Strauss wrote:
> On Tue, Jun 07, 2022 at 09:27:43AM -0700, Willy Tarreau wrote:
> > Hello Glenn,
> > 
> > Thanks for your report, I understand the problem, that's very interesting.
> > I would say it's not even an optim, rather an approach trying to help the
> > whole ecosystem at a very low cost by preventing known degenerative cases
> > from happening (or helping them recover).
> 
> Reference to issue (for the list)
> https://github.com/haproxy/haproxy/pull/1732
> https://github.com/nghttp2/nghttp2/issues/1722
> https://github.com/curl/curl/pull/8965

Yeah I've read those as well, thanks for the background and for reaching
out various implementations, by the way. You could even have brought the
discussion to the IETF HTTP working group to reach all of them at once,
and maybe someone would remember why there was this choice of 65535 back
then (I suspect it was done with IoT in mind where a window size had to
be stored on 16 bit to save space, but I could be wrong).

> Summary: degenerative HTTP/2 client behavior sending mostly 1-byte DATA
> frames for a fast client which continually exhausts the HTTP/2 send
> window (e.g. for a large file upload) *and* for a simple client which
> empties and reuses a single power-2 sized buffer.  A simple server
> sending WINDOW_UPDATE for each DATA frame to replenish the window may
> reflect back the exact size of the DATA frame.
> 
> > However I don't fully agree with the solution, it might just push the issue
> > a bit sidewards.
> 
> Changing the default SETTINGS_INITIAL_WINDOW_SIZE does push the issue a
> bit sideways, but in doing so mitigates a demonstrated (not theoretical)
> issue which might befall 6+ years of curl installations, a sizable
> number of which might not be upgraded for a very long time.
> 
> > For example a client might already have worked around this issue by using a
> > 65537-bytes buffer and will now face issues again.
> 
> I do not follow.  How might that be?
> Also, do you know of any such implementations?

No, it's just to illustrate. In the HTTP ecosystem, you rarely work
around one implementation's corner case without affecting another
workload. Or similarly, a client that would read exactly 65535 at
once could end up doing this with the change (please note that I'm
purposely making this up to illustrate):

 read 65535
 send 16384, 16384, 16384, 16383
 read 1
 send 1
 receive WU for 65535 (the first 4 frames)
 read 65535
 send 16384, 16384, 16384, 16383
 receive WU for 1
 read 1
 send 1
 ...

This would result in a client being less efficient by doubling the
number of read() syscalls. I'm not particularly worried about this,
to be honest, it's just that it's typically what could cause an issue
to be filed about an observed regression in field. In this case we'd
have to revert it and that would reopen the problem with curl.

> For a slightly more intelligent client actively trying to avoid the
> degenerative behavior, a simple approach is to avoid injecting tiny DATA
> frames when the window is larger and there is more data to be sent.  I
> have proposed a patch to curl in https://github.com/curl/curl/pull/8965

I totally agree, but we also know that it's harder to fix embedded
clients in cars and central heating devices than to deploy a
reasonable workaround on the few servers that receive their connections.

> > Thus I'd first prefer to solve it at the core, then possibly merge this
> > patch later as an optimization but nor for the problem, rather to make sure
> > clients can make 4 full 16kB frames, and improve bandwidth and CPU
> > efficiency.
> 
> Yes, having the server merge the sizes of small DATA frames into a
> larger WINDOW_UPDATE might be useful, too, but is independent and
> complimentary to my patch proposed in
> https://github.com/haproxy/haproxy/pull/1732

I agree that it's independent but it's the one that is not expected to
cause any regression with any possible client. That's why I'd like to
have the two. First that one because it should be durable. Second, your
patch as an optimization, and if anyone complains about trouble with it
we can safely revert it or decide what to do without worrying about all
deployed curl clients that will not be updated.

> > Given the nature of the problem, I don't imagine we'd meet this issue with
> > interleaved 1-byte frames from multiple streams over the same connection.
> 
> Actually, haproxy sets the session window size to be a very large
> number, effectively removing HTTP/2 application-level flow-control
> at the session level.  That leaves independent streams with the
> default SETTINGS_INITIAL_WINDOW_SIZE of 65535, so multiple uploads
> can independently exhaust the stream send window and run into this
> degenerative behavior provided the client is fast enough, the bandwidth
> delay product is large enough, and the client is fully emptying a
> power-2 sized buffer before reusing it.

Sure, but I'm speaking about