[PATCH 2/2] MINOR: connection: Transform safety check in PROXYv2 parsing into BUG_ON()

2022-02-25 Thread Tim Duesterhus
With BUG_ON() being enabled by default it is more useful to use a BUG_ON()
instead of an effectively never-taken if, as any incorrect assumptions will
become much more visible.

see 488ee7fb6e4a388bb68153341826a6391da794e9
---
 src/connection.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/connection.c b/src/connection.c
index f78028451..c156d9313 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -1098,12 +1098,11 @@ int conn_recv_proxy(struct connection *conn, int flag)
}
 
/* Verify that the PROXYv2 header ends at a TLV boundary.
-* This is technically unreachable, because the TLV parsing 
already
-* verifies that a TLV does not exceed the total length and also
-* that there is space for a TLV header.
+* This is can not be true, because the TLV parsing already
+* verifies that a TLV does not exceed the total length and
+* also that there is space for a TLV header.
 */
-   if (tlv_offset != total_v2_len)
-   goto bad_header;
+   BUG_ON(tlv_offset != total_v2_len);
 
/* unsupported protocol, keep local connection address */
break;
-- 
2.35.1




[PATCH 1/2] CLEANUP: connection: Indicate unreachability to the compiler in conn_recv_proxy

2022-02-25 Thread Tim Duesterhus
Transform the unreachability comment into a call to `my_unreachable()` to allow
the compiler from benefitting from it.

see d1b15b6e9b4d4d378a6169929a86f25b95eafc57
see 615f81eb5ad3e8c691901db8ce3e6a4a6b6efa49
---
 src/connection.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/src/connection.c b/src/connection.c
index 1d53eed79..f78028451 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -1073,9 +1073,8 @@ int conn_recv_proxy(struct connection *conn, int flag)
if (!isttest(conn->proxy_authority))
goto fail;
if (istcpy(>proxy_authority, tlv, 
PP2_AUTHORITY_MAX) < 0) {
-   /* This is technically unreachable, 
because we verified above
-* that the TLV value fits.
-*/
+   /* This is impossible, because we 
verified that the TLV value fits. */
+   my_unreachable();
goto fail;
}
break;
@@ -1087,9 +1086,8 @@ int conn_recv_proxy(struct connection *conn, int flag)
if (!isttest(conn->proxy_unique_id))
goto fail;
if (istcpy(>proxy_unique_id, tlv, 
UNIQUEID_LEN) < 0) {
-   /* This is technically unreachable, 
because we verified above
-* that the TLV value fits.
-*/
+   /* This is impossible, because we 
verified that the TLV value fits. */
+   my_unreachable();
goto fail;
}
break;
-- 
2.35.1




Re: Option to reject invalid percent encoded URLs?

2022-02-25 Thread Jesse Hathaway
On Fri, Feb 25, 2022 at 11:57 AM Tim Düsterhus  wrote:
> As far as I can tell currently this is only possible to achieve as a
> side-effect of the 'percent-*' normalizers of http-request normalize-uri:
>
>  http-request normalize-uri percent-to-uppercase strict
>
> will reject requests with invalid percent-encoding (and at the same time
> also uppercase the A-F characters in percent encoding).

Thanks Tim, exactly what I was looking for!



Re: Option to reject invalid percent encoded URLs?

2022-02-25 Thread Tim Düsterhus

Jesse,

On 2/25/22 18:25, Jesse Hathaway wrote:

invalid percent encoded URLs with a 400 Bad Request. Is there an option
to have HAProxy reject them as well, I couldn't find one in the docs,


As far as I can tell currently this is only possible to achieve as a 
side-effect of the 'percent-*' normalizers of http-request normalize-uri:


http-request normalize-uri percent-to-uppercase strict

will reject requests with invalid percent-encoding (and at the same time 
also uppercase the A-F characters in percent encoding).


Best regards
Tim Düsterhus



Option to reject invalid percent encoded URLs?

2022-02-25 Thread Jesse Hathaway
HAProxy accepts invalid percent encoded URLs, such as
http://example.com/%, whereas some web servers, such as nginx, reject
invalid percent encoded URLs with a 400 Bad Request. Is there an option
to have HAProxy reject them as well, I couldn't find one in the docs,
though accept-invalid-http-request seems to imply HAProxy would reject
invalid URLs by default?

Yours kindly, Jesse Hathaway



[ANNOUNCE] haproxy-2.6-dev2

2022-02-25 Thread Willy Tarreau
Hi,

HAProxy 2.6-dev2 was released on 2022/02/25. It added 212 new commits
after version 2.6-dev1.

First, there was the usual bag of bug fixes for such a dev release
(~35), as well as a few build fixes. As usual there are ~50 updates to
QUIC; certain small but annoying internal architecture limitations are
currently being addressed, so I wouldn't be surprised to see a steady
flow of patches in that area in the next releases.

The main focus for this version is:

  - end of the migration to OpenSSL 3.0's native API, except for the
engines, which will continue to work with the old API for now, and
will need to be architected differently for the new one (probably
long and tedious work, not even sure if any engine adopted that yet).

  - first pass of conn_stream rework. The addition of the muxes in 1.8
forced us to insert new layers in order not to break existing stuff
but these ones need to be progressively remerged with other adjacent
layers to reduce the risks of bugs bugs, improve performance and
maintainability. The goal is to pass through less layers between muxes
and streams so that we can get back to a more linear architecture like
we had in 1.6 with more direct communications between layers. Easier
said than done, but at least that first series now looked stable
enough to be merged instead of being continually rebased. More work
is expected there in the forthcoming weeks, so that we have a solid
base that will help backport fixes from 2.7-dev to 2.6 later without
taking risks.

  - the master CLI now supports a debug mode via "mcli-debug-mode on"
that allows all regular CLI commands to be visible and accessible
for the master process, in order to debug it (e.g. see connections
to workers etc). Expert-mode is also available and a set of flags
indicating the current mode are now displayed in the prompt.

  - httpclient updates, to support forcing a destination and setting a
transfer timeout.

  - improved debugging of the memory pools: now instead of fiddling with
build-time options to enable/disable certain debugging features, it
becomes possible to enable/disable them from the command line by
passing some memory debugging keywords after "-dM". The build-time
options still exist and only fix the default settings.

  - small improvements to the BUG_ON() macro and its cousins. More are
expected to come soon so that we can start to stuff them at plenty of
other places in the code and allow the level of validation to be
adjusted at build time, and more importantly that the reasonable
checks remain active by default.

  - basic pool debugging and first-level BUG_ON() are now enabled by
default in the makefile.

  - the CI now runs with full debugging turned on, and Coverity now also
covers QUIC

  - a few new regtests, mainly for SSL stuff.

I sincerely hope that the long series of painful bugs that kept us away
from coding is far behind and that we'll soon be able to add more entries
above. Stable branches 2.3 and older should see a release next week I
think.

Please find the usual URLs below :
   Site index   : http://www.haproxy.org/
   Discourse: http://discourse.haproxy.org/
   Slack channel: https://slack.haproxy.org/
   Issue tracker: https://github.com/haproxy/haproxy/issues
   Wiki : https://github.com/haproxy/wiki/wiki
   Sources  : http://www.haproxy.org/download/2.6/src/
   Git repository   : http://git.haproxy.org/git/haproxy.git/
   Git Web browsing : http://git.haproxy.org/?p=haproxy.git
   Changelog: http://www.haproxy.org/download/2.6/src/CHANGELOG
   Cyril's HTML doc : http://cbonte.github.io/haproxy-dconv/

Willy
---
Complete changelog :
Amaury Denoyelle (30):
  MINOR: h3: hardcode the stream id of control stream
  MINOR: mux-quic: remove quic_transport_params_update
  MINOR: quic: rename local tid variable
  MINOR: quic: remove unused xprt rcv_buf operation
  MINOR: quic: take out xprt snd_buf operation
  MINOR: quic: use a global dghlrs for each thread
  BUG/MEDIUM: quic: fix crash on CC if mux not present
  MINOR: qpack: fix typo in trace
  BUG/MINOR: quic: fix FIN stream signaling
  BUG/MINOR: h3: fix the header length for QPACK decoding
  MINOR: h3: remove transfer-encoding header
  MINOR: h3: add documentation on h3_decode_qcs
  MINOR: h3: set properly HTX EOM/BODYLESS on HEADERS parsing
  MINOR: mux-quic: implement rcv_buf
  MINOR: mux-quic: set EOS on rcv_buf
  MINOR: h3: set CS_FL_NOT_FIRST
  MINOR: h3: report frames bigger than rx buffer
  MINOR: h3: extract HEADERS parsing in a dedicated function
  MINOR: h3: implement DATA parsing
  MINOR: h3: report error on HEADERS/DATA parsing
  MINOR: h3: remove unused return value on decode_qcs
  MINOR: mux-quic: fix a possible null dereference in qc_timeout_task
  MINOR: quic: do not 

[ANNOUNCE] haproxy-2.4.14

2022-02-25 Thread Christopher Faulet

Hi,

HAProxy 2.4.14 was released on 2022/02/25. It added 26 new commits
after version 2.4.13.

The main issues fixed in this version are:

  - A major issue in the H2 multiplexer. An error during the response
processing, after the HEADERS frame parsing, led to a wakeup loop
consuming all the CPU because the error was not properly reported to the
upper layer. For instance, this happened if an invalid header value, an
invalid status code or a forbidden header was found in the
response. Note that only HAProxy >= 2.4 are affected by this issue.

  - A FD leak on reload failures. When the master process is reloaded on a
new config, it will try to connect to the previous process' socket to
retrieve all known listening FDs to be reused by the new listeners. If
listeners were removed, their unused FDs are simply closed. However
there's a catch. In case a socket fails to bind, the master will cancel
its startup and switch to wait mode for a new operation to happen. In
this case it didn't close the possibly remaining FDs that were left
unused.

  - A FD leak of a sockpair upon a failed reload.  When starting HAProxy in
master-worker, the master pre-allocate a struct mworker_proc and do a
socketpair() before the configuration parsing. If the configuration
loading failed, the FD was never closed because they aren't part of
listener, they are not even in the fdtab.

  - Some issues about errors on buffers allocation. First, in the H1
multiplexer. If we failed to send data because we failed to allocate the
H1 output buffer, the H1 stream was erroneously woken up. This led to a
wakeup loop to send more data while it is not possible because there is
no output buffer. Then, in process_stream(), if we failed to allocate
the channel response buffer while a connect or an analysis timeout
occurred, the stream was woken up in loop because its task was requeued
with an expired date. Now an error is reported when this happens and the
stream processing is interrupted.

Note there is a mechanism to deal with errors on buffers allocation.
Unfortunately, since the 1.7, this mechanism is broken. And it is even
worse now with the multiplexers. All this part must be refactored. But
for now, HAProxy may be partially frozen if too many entities are
waiting for a buffer.

  - Some alignment problems that were found when using gcc-11 + RHEL8,
resulting in instant crashes on startup.

  - An issue with multi-line ESMTP response in the mailer code.

  - An issue in the resolvers code with domain names with a trailing dot. The
trailing dot was not ignored as expected and a junk character was added
at the end of the encoded part of the domain name.

The remaining is the usual bunch of fixes and improvements. As usual, people
using the 2.4 branch are encouraged to migrate to this version.

Thanks everyone for your help and your contributions!

Please find the usual URLs below :
   Site index   : http://www.haproxy.org/
   Discourse: http://discourse.haproxy.org/
   Slack channel: https://slack.haproxy.org/
   Issue tracker: https://github.com/haproxy/haproxy/issues
   Wiki : https://github.com/haproxy/wiki/wiki
   Sources  : http://www.haproxy.org/download/2.4/src/
   Git repository   : http://git.haproxy.org/git/haproxy-2.4.git/
   Git Web browsing : http://git.haproxy.org/?p=haproxy-2.4.git
   Changelog: http://www.haproxy.org/download/2.4/src/CHANGELOG
   Cyril's HTML doc : http://cbonte.github.io/haproxy-dconv/


---
Complete changelog :
Christopher Faulet (6):
  BUG/MINOR: sink: Use the right field in appctx context in release callback
  BUG/MEDIUM: resolvers: Really ignore trailing dot in domain names
  BUG/MEDIUM: htx: Be sure to have a buffer to perform a raw copy of a 
message
  BUG/MEDIUM: mux-h1: Don't wake h1s if mux is blocked on lack of output 
buffer
  BUG/MAJOR: mux-h2: Be sure to always report HTX parsing error to the app 
layer
  BUG/MEDIUM: stream: Abort processing if response buffer allocation fails

Ilya Shipitsin (4):
  BUILD: adopt script/build-ssl.sh for OpenSSL-3.0.0beta2
  CI: github actions: add OpenSSL-3.0.0 builds
  CI: github actions: relax OpenSSL-3.0.0 version comparision
  CI: github actions: update OpenSSL to 3.0.1

Lukas Tribus (1):
  BUG/MINOR: mailers: negotiate SMTP, not ESMTP

William Lallemand (5):
  BUG/MINOR: mworker: fix a FD leak of a sockpair upon a failed reload
  BUILD: fix compilation for OpenSSL-3.0.0-alpha17
  CI: github actions: -Wno-deprecated-declarations with OpenSSL 3.0.0
  CI: github: switch to OpenSSL 3.0.0
  BUG/MINOR: tools: url2sa reads ipv4 too far

Willy Tarreau (10):
  MINOR: sock: move the unused socket cleaning code into its own function
  BUG/MEDIUM: mworker: close unused transferred FDs on load failure
  BUG/MEDIUM: fd: always 

[ANNOUNCE] haproxy-2.5.4

2022-02-25 Thread Christopher Faulet

Hi,

HAProxy 2.5.4 was released on 2022/02/25. It added 8 new commits
after version 2.5.3.

A major issue in the H2 multiplexer was fixed in this release. An error
during the response processing, after the HEADERS frame parsing, led to a
wakeup loop consuming all the CPU because the error was not properly
reported to the upper layer. For instance, this happened if an invalid
header value, an invalid status code or a forbidden header was found in the
response. Note that only HAProxy >= 2.4 are affected by this issue.

In addition, some issues about errors on buffers allocation were
fixed. First, in the H1 multiplexer. If we failed to send data because we
failed to allocate the H1 output buffer, the H1 stream was erroneously woken
up. This led to a wakeup loop to send more data while it is not possible
because there is no output buffer. Then, in process_stream(), if we failed
to allocate the channel response buffer while a connect or an analysis
timeout occurred, the stream was woken up in loop because its task was
requeued with an expired date. Now an error is reported when this happens
and the stream processing is interrupted.

Note there is a mechanism to deal with errors on buffers allocation.
Unfortunately, since the 1.7, this mechanism is broken. And it is even worse
now with the multiplexers. All this part must be refactored. But for now,
HAProxy may be partially Frozen if too many entities are waiting for a
buffer.

Other commits are doc improvements and small fixes here and there. As usual,
people using the 2.5 branch are encouraged to migrate to this version.

Thanks everyone for your help and your contributions!

Please find the usual URLs below :
   Site index   : http://www.haproxy.org/
   Discourse: http://discourse.haproxy.org/
   Slack channel: https://slack.haproxy.org/
   Issue tracker: https://github.com/haproxy/haproxy/issues
   Wiki : https://github.com/haproxy/wiki/wiki
   Sources  : http://www.haproxy.org/download/2.5/src/
   Git repository   : http://git.haproxy.org/git/haproxy-2.5.git/
   Git Web browsing : http://git.haproxy.org/?p=haproxy-2.5.git
   Changelog: http://www.haproxy.org/download/2.5/src/CHANGELOG
   Cyril's HTML doc : http://cbonte.github.io/haproxy-dconv/


---
Complete changelog :
Christian Ruppert (1):
  DOC: Fix usage/examples of deprecated ACLs

Christopher Faulet (4):
  BUG/MEDIUM: htx: Be sure to have a buffer to perform a raw copy of a 
message
  BUG/MEDIUM: mux-h1: Don't wake h1s if mux is blocked on lack of output 
buffer
  BUG/MAJOR: mux-h2: Be sure to always report HTX parsing error to the app 
layer
  BUG/MEDIUM: stream: Abort processing if response buffer allocation fails

Willy Tarreau (3):
  BUG/MINOR: proxy: preset the error message pointer to NULL in 
parse_new_proxy()
  REGTESTS: fix the race conditions in 40be_2srv_odd_health_checks
  CI: github: enable pool debugging by default

--
Christopher Faulet



Re: [PATCH] MINOR: sample: Add srv_rtt server round trip time sample

2022-02-25 Thread Willy Tarreau
On Fri, Feb 25, 2022 at 03:39:02PM +0100, Aleksandar Lazic wrote:
> > > diff --git a/reg-tests/sample_fetches/srv_rtt.vtc 
> > > b/reg-tests/sample_fetches/srv_rtt.vtc
> > > new file mode 100644
> > > index 0..c0ad0cbae
> > > --- /dev/null
> > > +++ b/reg-tests/sample_fetches/srv_rtt.vtc
> > > @@ -0,0 +1,34 @@
> > > +varnishtest "srv_rtt sample fetch Test"
> > > +
> > > +#REQUIRE_VERSION=2.6
> > 
> > Note, we *might* need to add a new macro to detect support for TCP_INFO.
> > We still don't have the config predicates to detect support for certain
> > keywords or sample fetch functions so that's not easy, but it's possible
> > that this test will break on some OS like cygwin. If so we could work
> > around this temporarily using "EXCLUDE_TARGETS" and in the worst case we
> > could mark it broken for the time it takes to completely solve this.
> 
> Agree here the suggestion is to add something like USE_GETSOCKOPT to be able
> to make '#REQUIRE_OPTIONS=GETSOCKOPT', something like similar to
> USE_GETADDRINFO, right?

No, it's not even this, it would in fact need to detect TCP_INFO which
comes from the system's includes. That's why we won't do that and instead
we'll later finish to add more predicates to detect supported config
keywords. This way if we mention a dependency on "bc_rtt", we know we
can skip this test on systems which don't include it based on haproxy's
internal features detection at build time.

Do not worry for this one, leave it as-is, we'll see what breaks on the
CI (likely only cygwin) and we'll exclude it/them.

Cheers,
Willy



Re: [PATCH] MINOR: sample: Add srv_rtt server round trip time sample

2022-02-25 Thread Aleksandar Lazic



Hi Willy.

On 25.02.22 14:54, Willy Tarreau wrote:

Hi Alex,

On Thu, Feb 24, 2022 at 03:03:59AM +0100, Aleksandar Lazic wrote:

Hi.

Here the first patch for feature request "New Balancing algorithm (Peak) EWMA 
#1570"


Note, I don't think it is needed for this algo as long as we instead
use measured response time and/or health check time. But regardless
it's something useful to have. A few comments below:



Thanks you for your much valuable feedback.
I think also that the rtt information as fetch sample could be useful.


 From e95bf6a4bf107fdc59696c4b4a4ef7b03133b813 Mon Sep 17 00:00:00 2001
From: Aleksandar Lazic 
Date: Thu, 24 Feb 2022 02:56:21 +0100
Subject: [PATCH] MINOR: sample: Add srv_rtt server round trip time sample

This sample fetch get the server round trip time


You should mention "TCP round trip time" since it's measured at the TCP
level.


+srv_rtt : integer
+  Returns the Round Trip Time (RTT) measured by the kernel for the server
+  connection.  is facultative, by default the unit is milliseconds. 

+  can be set to "ms" for milliseconds or "us" for microseconds. If the server
+  connection is not established, if the connection is not TCP or if the
+  operating system does not support TCP_INFO, for example Linux kernels before
+  2.4, the sample fetch fails.


I would rather call it "bc_rtt" since it's not the server but the backend
connection. Technically speaking it indeed requires a connection to be
established and will only report the value for *this* connection and not
anything stateless related to the server. Thta's more in line with what we
have for the frontend connection already with fc_rtt.

You mentioned "unit" but it does not appear in the keyword syntax.

Also I think it would be useful to have all other fc_* from the same
section turned to bc_* (fc_rttvar, fc_retrans, etc), as it can sometimes
explain long response times in logs.


diff --git a/reg-tests/sample_fetches/srv_rtt.vtc 
b/reg-tests/sample_fetches/srv_rtt.vtc
new file mode 100644
index 0..c0ad0cbae
--- /dev/null
+++ b/reg-tests/sample_fetches/srv_rtt.vtc
@@ -0,0 +1,34 @@
+varnishtest "srv_rtt sample fetch Test"
+
+#REQUIRE_VERSION=2.6


Note, we *might* need to add a new macro to detect support for TCP_INFO.
We still don't have the config predicates to detect support for certain
keywords or sample fetch functions so that's not easy, but it's possible
that this test will break on some OS like cygwin. If so we could work
around this temporarily using "EXCLUDE_TARGETS" and in the worst case we
could mark it broken for the time it takes to completely solve this.


Agree here the suggestion is to add something like USE_GETSOCKOPT to be able to 
make '#REQUIRE_OPTIONS=GETSOCKOPT',
something like similar to USE_GETADDRINFO, right?


(...)

diff --git a/src/tcp_sample.c b/src/tcp_sample.c
index 19edcd243..7b8b616cb 100644
--- a/src/tcp_sample.c
+++ b/src/tcp_sample.c
@@ -446,6 +446,20 @@ smp_fetch_fc_reordering(const struct arg *args, struct 
sample *smp, const char *
return 0;
return 1;
  }
+
+/* get the mean rtt of a client connection */
+static int
+smp_fetch_srv_rtt(const struct arg *args, struct sample *smp, const char *kw, 
void *private)
+{
+   if (!get_tcp_info(args, smp, 1, 0))
+   return 0;
+
+   /* By default or if explicitly specified, convert rtt to ms */
+   if (!args || args[0].type == ARGT_STOP || args[0].data.sint == 
TIME_UNIT_MS)
+   smp->data.u.sint = (smp->data.u.sint + 500) / 1000;
+
+   return 1;
+}


That's another reason for extending the existing keywords, avoiding code
duplication. You can have all your new keywords map to the fc_* equivalent
and just change this:

  - if (!get_tcp_info(args, smp, 0, 0))
  + if (!get_tcp_info(args, smp, *kw == 'b', 0))

Please update the comments on top of the functions to mention that they're
called for both "fc_*" and "bc_*" depending on the side, and that's OK.


Let me go back to the "drawing board" and will send a update as soon as there 
is a update :-)


thanks,
Willy


Regards
Alex



Re: [PATCH] MINOR: sample: Add srv_rtt server round trip time sample

2022-02-25 Thread Willy Tarreau
Hi Alex,

On Thu, Feb 24, 2022 at 03:03:59AM +0100, Aleksandar Lazic wrote:
> Hi.
> 
> Here the first patch for feature request "New Balancing algorithm (Peak) EWMA 
> #1570"

Note, I don't think it is needed for this algo as long as we instead
use measured response time and/or health check time. But regardless
it's something useful to have. A few comments below:

> From e95bf6a4bf107fdc59696c4b4a4ef7b03133b813 Mon Sep 17 00:00:00 2001
> From: Aleksandar Lazic 
> Date: Thu, 24 Feb 2022 02:56:21 +0100
> Subject: [PATCH] MINOR: sample: Add srv_rtt server round trip time sample
> 
> This sample fetch get the server round trip time

You should mention "TCP round trip time" since it's measured at the TCP
level.

> +srv_rtt : integer
> +  Returns the Round Trip Time (RTT) measured by the kernel for the server
> +  connection.  is facultative, by default the unit is milliseconds. 
> 
> +  can be set to "ms" for milliseconds or "us" for microseconds. If the server
> +  connection is not established, if the connection is not TCP or if the
> +  operating system does not support TCP_INFO, for example Linux kernels 
> before
> +  2.4, the sample fetch fails.

I would rather call it "bc_rtt" since it's not the server but the backend
connection. Technically speaking it indeed requires a connection to be
established and will only report the value for *this* connection and not
anything stateless related to the server. Thta's more in line with what we
have for the frontend connection already with fc_rtt.

You mentioned "unit" but it does not appear in the keyword syntax.

Also I think it would be useful to have all other fc_* from the same
section turned to bc_* (fc_rttvar, fc_retrans, etc), as it can sometimes
explain long response times in logs.

> diff --git a/reg-tests/sample_fetches/srv_rtt.vtc 
> b/reg-tests/sample_fetches/srv_rtt.vtc
> new file mode 100644
> index 0..c0ad0cbae
> --- /dev/null
> +++ b/reg-tests/sample_fetches/srv_rtt.vtc
> @@ -0,0 +1,34 @@
> +varnishtest "srv_rtt sample fetch Test"
> +
> +#REQUIRE_VERSION=2.6

Note, we *might* need to add a new macro to detect support for TCP_INFO.
We still don't have the config predicates to detect support for certain
keywords or sample fetch functions so that's not easy, but it's possible
that this test will break on some OS like cygwin. If so we could work
around this temporarily using "EXCLUDE_TARGETS" and in the worst case we
could mark it broken for the time it takes to completely solve this.

(...)
> diff --git a/src/tcp_sample.c b/src/tcp_sample.c
> index 19edcd243..7b8b616cb 100644
> --- a/src/tcp_sample.c
> +++ b/src/tcp_sample.c
> @@ -446,6 +446,20 @@ smp_fetch_fc_reordering(const struct arg *args, struct 
> sample *smp, const char *
>   return 0;
>   return 1;
>  }
> +
> +/* get the mean rtt of a client connection */
> +static int
> +smp_fetch_srv_rtt(const struct arg *args, struct sample *smp, const char 
> *kw, void *private)
> +{
> + if (!get_tcp_info(args, smp, 1, 0))
> + return 0;
> +
> + /* By default or if explicitly specified, convert rtt to ms */
> + if (!args || args[0].type == ARGT_STOP || args[0].data.sint == 
> TIME_UNIT_MS)
> + smp->data.u.sint = (smp->data.u.sint + 500) / 1000;
> +
> + return 1;
> +}

That's another reason for extending the existing keywords, avoiding code
duplication. You can have all your new keywords map to the fc_* equivalent
and just change this:

 -  if (!get_tcp_info(args, smp, 0, 0))
 +  if (!get_tcp_info(args, smp, *kw == 'b', 0))

Please update the comments on top of the functions to mention that they're
called for both "fc_*" and "bc_*" depending on the side, and that's OK.

thanks,
Willy