Log lines in 2.0

[Igor Cicimov]

Hi,

I have an HTTP frontend running on specific PORT for the purpose of
external health checks, so typical:

mode http
option httplog

I noticed the log lines though for haproxy v2.0.13 I installed from the
usual Ubuntu PPA from Vincent:

# haproxy -v
HA-Proxy version 2.0.13-1ppa1~bionic 2020/02/15 - https://haproxy.org/

are different from what I'm used to seeing in v1.8 lets say, looks like
some extra lines for the headers are being logged?

Feb 27 03:37:21 ip-10-0-4-33 haproxy[21361]:
0d56:monitor-in.accept(0009)=0012 from [IP:56142] ALPN=
Feb 27 03:37:21 ip-10-0-4-33 haproxy[21361]:
0d56:monitor-in.clireq[0012:]: GET /monitor-url HTTP/1.1
Feb 27 03:37:21 ip-10-0-4-33 haproxy[21361]:
0d56:monitor-in.clihdr[0012:]: host: 10.0.4.33:PORT
Feb 27 03:37:21 ip-10-0-4-33 haproxy[21361]:
0d56:monitor-in.clihdr[0012:]: user-agent: ELB-HealthChecker/1.0
Feb 27 03:37:21 ip-10-0-4-33 haproxy[21361]:
0d56:monitor-in.clihdr[0012:]: accept: */*
Feb 27 03:37:21 ip-10-0-4-33 haproxy[21361]:
0d56:monitor-in.clicls[0012:]
Feb 27 03:37:21 ip-10-0-4-33 haproxy[21361]:
0d56:monitor-in.closed[0012:]

I don't have any log-format settings thus the default ones should be in
play so wonder if this is what I should see?

Thanks,
Igor

Re: [PATCH 1/3] BUG/MINOR: sample: Make sure to return stable IDs in the unique-id fetch

[Willy Tarreau]

On Wed, Feb 26, 2020 at 04:20:49PM +0100, Tim Duesterhus wrote:
> Previously when the `unique-id-format` contained non-deterministic parts,
> such as the `uuid` fetch each use of the `unique-id` fetch would generate
> a new unique ID, replacing the old one. The following configuration shows
> the error:
(...)

Good catch, and now applied, thanks!
Willy

Re: [PATCH 2/3] MINOR: stream: Add stream_generate_unique_id function

[Willy Tarreau]

On Wed, Feb 26, 2020 at 04:20:50PM +0100, Tim Duesterhus wrote:
> Currently unique IDs for a stream are generated using repetitive code in
> multiple locations, possibly allowing for inconsistent behavior.

Just a few minor coding style comments :

> +int stream_generate_unique_id(struct stream *strm, struct list *format) {

Please put the function's opening brace on its own line. The reason for
this is when you have many arguments and variables, it easily becomes a
mess where you cannot always visually tell which ones belong to what.

> + if (strm->unique_id != NULL) {
> + return strlen(strm->unique_id);
> + }
> +
> + if ((strm->unique_id = pool_alloc(pool_head_uniqueid)) == NULL)
> + return -1;

Please avoid assignments in "if" conditions. There are two reasons for
this, both related to debugging:
   - if you want to quickly disable the error check or put a lock
 around or whatever, you cannot without splitting the line in
 two ;

   - you often cannot single-step through it in a debugger or put a
 breakpoint after the pool_alloc() call.

Thanks,
Willy

Re: [PATCH 3/3] MINOR: stream: Use stream_generate_unique_id

[Willy Tarreau]

On Wed, Feb 26, 2020 at 04:20:51PM +0100, Tim Duesterhus wrote:
> This patch replaces the ad-hoc generation of stream's `unique_id` values
> by calls to `stream_generate_unique_id`.

It seems to me that it won't generate the unique_id anymore if there
is no unique-id-header directive in the config :

>   http_manage_client_side_cookies(s, req);
>  
>   /* add unique-id if "header-unique-id" is specified */
> + if (sess->fe->header_unique_id && 
> !LIST_ISEMPTY(>fe->format_unique_id)) {
^^
All the unique-id generation seems to be enclosed in this. Am I missing
something ?

(...)

Other than that I have a suggestion, I've seen recently in this thread
a few calls to strlen() on unique_id and header_unique_id. I think they
should be turned to ist so that the length is always stored with them
and we don't need to run strlen on them anymore at runtime. And this
will simplify the header addition which will basically look like this:

 http_add_header(htx, sess->fe->header_unique_id, s->unique_id);

Willy

[PATCH 1/3] BUG/MINOR: sample: Make sure to return stable IDs in the unique-id fetch

[Tim Duesterhus]

Previously when the `unique-id-format` contained non-deterministic parts,
such as the `uuid` fetch each use of the `unique-id` fetch would generate
a new unique ID, replacing the old one. The following configuration shows
the error:

  global
log stdout format short daemon

  listen test
log global
log-format "%ID"
unique-id-format %{+X}o\ TEST-%[uuid]

mode http
bind *:8080
http-response set-header A %[unique-id]
http-response set-header B %[unique-id]
server example example.com:80

Without the patch the contents of the `A` and `B` response header would
differ.

This bug was introduced in commit f4011ddcf5b41284d2b137e84c25f2d1264ce458,
which was first released with HAProxy 1.7-dev3.

This fix should be backported to HAProxy 1.7+.
---
 src/http_fetch.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/http_fetch.c b/src/http_fetch.c
index 29a5d17d8..d288e841d 100644
--- a/src/http_fetch.c
+++ b/src/http_fetch.c
@@ -416,10 +416,10 @@ static int smp_fetch_uniqueid(const struct arg *args, 
struct sample *smp, const
if ((smp->strm->unique_id = pool_alloc(pool_head_uniqueid)) == 
NULL)
return 0;
smp->strm->unique_id[0] = '\0';
+   build_logline(smp->strm, smp->strm->unique_id,
+ UNIQUEID_LEN, >sess->fe->format_unique_id);
}
-   smp->data.u.str.data = build_logline(smp->strm, smp->strm->unique_id,
-   UNIQUEID_LEN, 
>sess->fe->format_unique_id);
-
+   smp->data.u.str.data = strlen(smp->strm->unique_id);
smp->data.type = SMP_T_STR;
smp->data.u.str.area = smp->strm->unique_id;
smp->flags = SMP_F_CONST;
-- 
2.25.1

[PATCH 3/3] MINOR: stream: Use stream_generate_unique_id

[Tim Duesterhus]

This patch replaces the ad-hoc generation of stream's `unique_id` values
by calls to `stream_generate_unique_id`.
---
 src/http_ana.c   | 14 ++
 src/http_fetch.c | 17 -
 src/log.c|  3 +--
 3 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/src/http_ana.c b/src/http_ana.c
index 20c7b6e50..d6f41b428 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -788,20 +788,18 @@ int http_process_request(struct stream *s, struct channel 
*req, int an_bit)
http_manage_client_side_cookies(s, req);
 
/* add unique-id if "header-unique-id" is specified */
+   if (sess->fe->header_unique_id && 
!LIST_ISEMPTY(>fe->format_unique_id)) {
+   struct ist n, v;
+   int length;
 
-   if (!LIST_ISEMPTY(>fe->format_unique_id) && !s->unique_id) {
-   if ((s->unique_id = pool_alloc(pool_head_uniqueid)) == NULL) {
+   if ((length = stream_generate_unique_id(s, 
>fe->format_unique_id)) < 0) {
if (!(s->flags & SF_ERR_MASK))
s->flags |= SF_ERR_RESOURCE;
goto return_int_err;
}
-   s->unique_id[0] = '\0';
-   build_logline(s, s->unique_id, UNIQUEID_LEN, 
>fe->format_unique_id);
-   }
 
-   if (sess->fe->header_unique_id && s->unique_id) {
-   struct ist n = ist2(sess->fe->header_unique_id, 
strlen(sess->fe->header_unique_id));
-   struct ist v = ist2(s->unique_id, strlen(s->unique_id));
+   n = ist2(sess->fe->header_unique_id, 
strlen(sess->fe->header_unique_id));
+   v = ist2(s->unique_id, length);
 
if (unlikely(!http_add_header(htx, n, v)))
goto return_int_err;
diff --git a/src/http_fetch.c b/src/http_fetch.c
index d288e841d..dbbb5ecfd 100644
--- a/src/http_fetch.c
+++ b/src/http_fetch.c
@@ -409,19 +409,18 @@ static int smp_fetch_stcode(const struct arg *args, 
struct sample *smp, const ch
 
 static int smp_fetch_uniqueid(const struct arg *args, struct sample *smp, 
const char *kw, void *private)
 {
+   int length;
+
if (LIST_ISEMPTY(>sess->fe->format_unique_id))
return 0;
 
-   if (!smp->strm->unique_id) {
-   if ((smp->strm->unique_id = pool_alloc(pool_head_uniqueid)) == 
NULL)
-   return 0;
-   smp->strm->unique_id[0] = '\0';
-   build_logline(smp->strm, smp->strm->unique_id,
- UNIQUEID_LEN, >sess->fe->format_unique_id);
-   }
-   smp->data.u.str.data = strlen(smp->strm->unique_id);
-   smp->data.type = SMP_T_STR;
+   length = stream_generate_unique_id(smp->strm, 
>sess->fe->format_unique_id);
+   if (length < 0)
+   return 0;
+
smp->data.u.str.area = smp->strm->unique_id;
+   smp->data.u.str.data = length;
+   smp->data.type = SMP_T_STR;
smp->flags = SMP_F_CONST;
return 1;
 }
diff --git a/src/log.c b/src/log.c
index 60b1a5a4d..b46605b8d 100644
--- a/src/log.c
+++ b/src/log.c
@@ -2983,8 +2983,7 @@ void strm_log(struct stream *s)
 
/* if unique-id was not generated */
if (!s->unique_id && !LIST_ISEMPTY(>fe->format_unique_id)) {
-   if ((s->unique_id = pool_alloc(pool_head_uniqueid)) != NULL)
-   build_logline(s, s->unique_id, UNIQUEID_LEN, 
>fe->format_unique_id);
+   stream_generate_unique_id(s, >fe->format_unique_id);
}
 
if (!LIST_ISEMPTY(>fe->logformat_sd)) {
-- 
2.25.1

[PATCH 0/3] Add helper function to generate stream unique IDs

[Tim Duesterhus]

Willy,

during my investigation regarding unique IDs for TCP streams I noticed that
the `unique-id` sample fetch had a bug. Thus I spun out the first pieces of
my work into this patchset. The first patch fixes the bug in the fetch. The
other patches clean up the repetitive generation of the unique id. Not sure
whether you might want to re-tag the last to CLEANUP.

Best regards

Tim Duesterhus (3):
  BUG/MINOR: sample: Make sure to return stable IDs in the unique-id
fetch
  MINOR: stream: Add stream_generate_unique_id function
  MINOR: stream: Use stream_generate_unique_id

 include/proto/stream.h |  3 +++
 src/http_ana.c | 15 ++-
 src/http_fetch.c   | 15 +++
 src/log.c  |  3 +--
 src/stream.c   | 19 +++
 5 files changed, 36 insertions(+), 19 deletions(-)

-- 
2.25.1

[PATCH 2/3] MINOR: stream: Add stream_generate_unique_id function

[Tim Duesterhus]

Currently unique IDs for a stream are generated using repetitive code in
multiple locations, possibly allowing for inconsistent behavior.
---
 include/proto/stream.h |  3 +++
 src/http_ana.c |  1 -
 src/stream.c   | 19 +++
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/include/proto/stream.h b/include/proto/stream.h
index f8c0887b9..e54ac60cc 100644
--- a/include/proto/stream.h
+++ b/include/proto/stream.h
@@ -53,6 +53,7 @@ extern struct trace_source trace_strm;
 #define IS_HTX_STRM(strm) ((strm)->flags & SF_HTX)
 
 extern struct pool_head *pool_head_stream;
+extern struct pool_head *pool_head_uniqueid;
 extern struct list streams;
 
 extern struct data_cb sess_conn_cb;
@@ -65,6 +66,8 @@ void stream_shutdown(struct stream *stream, int why);
 void stream_dump(struct buffer *buf, const struct stream *s, const char *pfx, 
char eol);
 void stream_dump_and_crash(enum obj_type *obj, int rate);
 
+int stream_generate_unique_id(struct stream *strm, struct list *format);
+
 void stream_process_counters(struct stream *s);
 void sess_change_server(struct stream *sess, struct server *newsrv);
 struct task *process_stream(struct task *t, void *context, unsigned short 
state);
diff --git a/src/http_ana.c b/src/http_ana.c
index e3d22445e..20c7b6e50 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -5093,7 +5093,6 @@ void http_end_txn(struct stream *s)
 
 
 DECLARE_POOL(pool_head_http_txn, "http_txn", sizeof(struct http_txn));
-DECLARE_POOL(pool_head_uniqueid, "uniqueid", UNIQUEID_LEN);
 
 __attribute__((constructor))
 static void __http_protocol_init(void)
diff --git a/src/stream.c b/src/stream.c
index 9798c5f0f..e0899b019 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -66,6 +66,7 @@
 #include 
 
 DECLARE_POOL(pool_head_stream, "stream", sizeof(struct stream));
+DECLARE_POOL(pool_head_uniqueid, "uniqueid", UNIQUEID_LEN);
 
 struct list streams = LIST_HEAD_INIT(streams);
 __decl_spinlock(streams_lock);
@@ -2657,6 +2658,24 @@ void stream_dump_and_crash(enum obj_type *obj, int rate)
abort();
 }
 
+/* Generates a unique ID based on the given , stores it in the given 
 and
+ * returns the length of the ID. -1 is returned on memory allocation failure.
+ *
+ * If an ID is already stored within the stream nothing happens and length of 
the stored
+ * ID is returned.
+ */
+int stream_generate_unique_id(struct stream *strm, struct list *format) {
+   if (strm->unique_id != NULL) {
+   return strlen(strm->unique_id);
+   }
+
+   if ((strm->unique_id = pool_alloc(pool_head_uniqueid)) == NULL)
+   return -1;
+   strm->unique_id[0] = 0;
+
+   return build_logline(strm, strm->unique_id, UNIQUEID_LEN, format);
+}
+
 //
 /*   All supported ACL keywords must be declared here.  */
 //
-- 
2.25.1

Re: [PATCH] supress cirrus-ci OS version check

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

I've adjusted commit message.

ср, 26 февр. 2020 г. в 12:40, Илья Шипицин :

> it's pretty much like to gcc.
> I've no idea why does FreeBSD decided to check its version (and nobody
> knows).
>
> I'll update commit message
>
> ср, 26 февр. 2020 г. в 05:37, Tim Düsterhus :
>
>> Ilya,
>>
>> Am 25.02.20 um 21:13 schrieb Илья Шипицин:
>> > it turned out that FreeBSD image is slightly different from packages.
>> > since we cannot change image, let us just suppress that warning.
>>
>> I'd like to point out this paragraph from the CONTRIBUTING document:
>>
>> >As a rule of thumb, your patch MUST NEVER be made only of a subject
>> line,
>> >it *must* contain a description. Even one or two lines, or indicating
>> >whether a backport is desired or not. It turns out that single-line
>> commits
>> >are so rare in the Git world that they require special manual (hence
>> >painful) handling when they are backported, and at least for this
>> reason
>> >it's important to keep this in mind.
>>
>> When reading your patch it was unclear to me which kind of version check
>> needs to be ignored (and why). At the very least I would have expected a
>> copy the actual error message from CI (that will be fixed after applying
>> the patch) in the commit message. I'll copy it here for posterity.
>>
>> > Updating FreeBSD repository catalogue...
>> > Fetching meta.conf: . done
>> > Fetching packagesite.txz: .. done
>> > Processing entries:
>> > Newer FreeBSD version for package p5-Bio-GFF3:
>> > To ignore this error set IGNORE_OSVERSION=yes
>> > - package: 1300081
>> > - running kernel: 1300078
>> > Ignore the mismatch and continue? [Y/n]: pkg: repository FreeBSD
>> contains packages for wrong OS version: FreeBSD:13:amd64
>> > Processing entries... done
>> > Unable to update repository FreeBSD
>> > Error updating repositories!
>>
>> I'm not a FreeBSD guy, but my understanding after a short session with
>> the search engine of my choice indicated that apparently the Cirrus CI
>> VM is running an outdated kernel or possibly an outdated FreeBSD jail.
>> I'm not sure whether that is correct or not. In any case an explanation
>> why suddenly this error message popped up and why it's safe to ignore
>> that specific error is safe would have been helpful in the commit
>> message as well.
>>
>> Best regards
>> Tim Düsterhus
>>
>
From 188581fda9a7110360f5bbbc7b56f9cda03ddd6c Mon Sep 17 00:00:00 2001
From: Ilya Shipitsin 
Date: Wed, 26 Feb 2020 19:29:36 +0500
Subject: [PATCH] BUILD: cirrus-ci: suppress OS version check when installing
 packages

since we run "snapshot" images of FreeBSD, it is possible that kernel
ABI version might change from time to time. It might differ from
prebuilt packages (installed via "pkg"). We do not test kernel modules,
so for us is safe to ignore ABI mismatch.
---
 .cirrus.yml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.cirrus.yml b/.cirrus.yml
index 1a07c80c7..86b404782 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -5,6 +5,8 @@ FreeBSD_task:
   image_family: freebsd-12-1-snap
   image_family: freebsd-11-3-snap
   only_if: $CIRRUS_BRANCH =~ 'master|next'
+  env:
+IGNORE_OSVERSION: yes # supress package installation error on FreeBSD-13
   install_script:
 - pkg update -f && pkg upgrade -y && pkg install -y openssl git gmake lua53 socat
   script:
-- 
2.24.1

Re: Unique ID for PROXYv2: Get 'struct stream' from 'struct connection'

[Willy Tarreau]

On Wed, Feb 26, 2020 at 12:25:52PM +0100, Tim Düsterhus wrote:
> Thank you. When I try this within `conn_si_send_proxy`it works as expected.
> 
> I have a question about the '} else if (!cs && conn->owner) {' case,
> though. It was added within this commit:
> https://github.com/haproxy/haproxy/commit/661167d136d5bf4ac40ac4fae450d66dd69a64df.
> The comment says something about SSL handshakes. However I am unable to
> trigger this condition. I tested with both SSL on the frontend bind line
> and on the backend server. Do you happen to know when I might not yet
> have a stream at the send proxy stage?

I don't exactly know but I think it's related to the zero-rtt mode
in TLS-1.3 because in this case we can get the request before the
handshake is complete. I think only Olivier knows how to trigger
this :-)

> Yes, my current implementation looks like that:
> 
> server example 127.0.0.1:8082 send-proxy-v2 proxy-v2-options unique-id

Looks clean like this.

> The 'explicit focus' is just that the user should not expect the IDs to
> make sense when using HTTP/2 with multiple streams over a single
> connections.

I agree.

> Yes, I plan to make adjustments for `accept-proxy` with a new sample
> fetch as well.

OK thank you!

Willy

Re: [PATCH 4/4] MINOR: ssl: "show ssl cert" command should print the "Chain filename:"

[William Lallemand]

On Wed, Feb 26, 2020 at 11:15:00AM +0100, Emmanuel Hocdet wrote:
> Hi,
> 
> > Le 18 févr. 2020 à 17:49, Emmanuel Hocdet  a écrit :
> >> 
> >> Yes. Show the chain-filename would be very helpful.
> >> For that i think a good way would be to keep ckch->chain and ckch->issuer
> >> with value (or NULL) from PEM/, and resolve chain and ocsp_issuer
> >> when needed. « show ssl cert » will be able to find the origin of chain 
> >> (and ocsp_issuer)
> >> without  store a new state. The drawback(?) is that .issuer file will be 
> >> loaded, in every case, if present.
> >> 
> > 
> > 
> > Patch series to do that:
> > 
> > example:
> > Issuer: /C=US/O=Let's Encrypt/CN=Let's Encrypt Authority X3
> > Chain filename: /etc/haproxy/issuers/letsencryptEC.pem
> > 
> 
> Rebased with current dev branch.
> 

Thanks, applied.

I made a cosmetic change in the "show ssl cert" output, and also
reworded the commit message a little bit.

-- 
William Lallemand

Re: Unique ID for PROXYv2: Get 'struct stream' from 'struct connection'

[Tim Düsterhus]

Willy,

Am 26.02.20 um 09:59 schrieb Willy Tarreau:
>>> However in many cases you could know the stream that requested the
>>> outgoing connection. That's what is used when you see that "remote"
>>> thing. In conn_si_send_proxy(), there's a test to see if the the
>>> first connection's conn_stream's owner is a stream_interface, and
>>> if so it uses si_opposite() to find the other side. From this
>>> stream interface you can get the requesting stream using si_strm().
>>
>> So I would do:
>>
>> struct stream *stream = si_strm(si_opposite(si));
> 
> You can even do :
> 
> struct stream *stream = si_strm(si);
> 
> Because the chain looks like this :
> 
> ++
>  conn--mux--cs--| si[0]   stream   si[1] |--cs--mux--conn
> ++
> FrontBack
> 
> When you do si_opposite(si[1]) it gives you si[0] and conversely.
> Both are part of the same struct stream. Thus si_strm() returns
> the same stream for any of them.

Thank you. When I try this within `conn_si_send_proxy`it works as expected.

I have a question about the '} else if (!cs && conn->owner) {' case,
though. It was added within this commit:
https://github.com/haproxy/haproxy/commit/661167d136d5bf4ac40ac4fae450d66dd69a64df.
The comment says something about SSL handshakes. However I am unable to
trigger this condition. I tested with both SSL on the frontend bind line
and on the backend server. Do you happen to know when I might not yet
have a stream at the send proxy stage?

>> Asking differently: If I wanted to upstream my proposal (with the
>> explicit focus on unique IDs for TCP), how would you like to see it work?
> 
> I don't know yet. I'm just a bit concerned about the risk of having
> something in TCP that disappears in HTTP if you do it for TCP only.
> Someone using this connection ID for logging (that's often the purpose)
> would only have it with TCP and would have to proceed differently for
> HTTP. So I'm still wondering if it wouldn't be better to rely on
> "proxy-v2-options" and decide what to send, and when the unique-id is
> specified there, then it's taken from the stream, regardless of the
> stream mode (tcp/http). Even if I consider that it doesn't make sense

Yes, my current implementation looks like that:

server example 127.0.0.1:8082 send-proxy-v2 proxy-v2-options unique-id

The 'explicit focus' is just that the user should not expect the IDs to
make sense when using HTTP/2 with multiple streams over a single
connections.

> to have this ID sent both on the connection and in a header, if there
> is a use case to having such an ID on a connection, we must still be
> sure that it doesn't become impossible to send it when in HTTP.
> 
> Also maybe having a way to retrieve a received ID on a listener as a
> sample fetch so as to be able to build a unique-id header from it
> could be useful. Some users want to pass the same ID along a whole
> chain, and with this it would become possible at the TCP level, which
> is quite nice.

Yes, I plan to make adjustments for `accept-proxy` with a new sample
fetch as well.

Best regards
Tim Düsterhus

Re: [PATCH 4/4] MINOR: ssl: "show ssl cert" command should print the "Chain filename:"

[Emmanuel Hocdet]

Hi,Le 18 févr. 2020 à 17:49, Emmanuel Hocdet  a écrit :Yes. Show the chain-filename would be very helpful.For that i think a good way would be to keep ckch->chain and ckch->issuerwith value (or NULL) from PEM/, and resolve chain and ocsp_issuerwhen needed. « show ssl cert » will be able to find the origin of chain (and ocsp_issuer)without  store a new state. The drawback(?) is that .issuer file will be loaded, in every case, if present.Patch series to do that:example:Issuer: /C=US/O=Let's Encrypt/CN=Let's Encrypt Authority X3Chain filename: /etc/haproxy/issuers/letsencryptEC.pemRebased with current dev branch.++Manu

0001-MINOR-ssl-move-find-certificate-chain-code-to-its-ow.patch
Description: Binary data


0002-MINOR-ssl-resolve-issuers-chain-later.patch
Description: Binary data


0003-MINOR-ssl-resolve-ocsp_issuer-later.patch
Description: Binary data


0004-MINOR-ssl-show-ssl-cert-command-should-print-the-Cha.patch
Description: Binary data

Re: Unique ID for PROXYv2: Get 'struct stream' from 'struct connection'

[Willy Tarreau]

On Tue, Feb 25, 2020 at 12:26:41PM +0100, Tim Düsterhus wrote:
> > You will not get back to a stream based on a connection because there
> > may be many streams per connection. A connection will lead you to a
> > mux (H1,H2,FCGI) and from there depending on the mux's protocol and
> > internals you'll have 0 to many streams above.
> 
> The specific use case I'm exploring is unique IDs for TCP proxying. I
> believe for TCP there should be a 1:1 mapping between connections and
> streams, no? Anything HTTP can and should just use the header.

Indeed for TCP it can make some sense to have something like this.

> > However in many cases you could know the stream that requested the
> > outgoing connection. That's what is used when you see that "remote"
> > thing. In conn_si_send_proxy(), there's a test to see if the the
> > first connection's conn_stream's owner is a stream_interface, and
> > if so it uses si_opposite() to find the other side. From this
> > stream interface you can get the requesting stream using si_strm().
> 
> So I would do:
> 
> struct stream *stream = si_strm(si_opposite(si));

You can even do :

struct stream *stream = si_strm(si);

Because the chain looks like this :

++
 conn--mux--cs--| si[0]   stream   si[1] |--cs--mux--conn
++
FrontBack

When you do si_opposite(si[1]) it gives you si[0] and conversely.
Both are part of the same struct stream. Thus si_strm() returns
the same stream for any of them.

> As mentioned above: TCP proxying instead of HTTP. Unless my
> understanding of 1 connection = 1 stream for TCP is incorrect.

No, it is correct.

> > Maybe you just need a per-connection unique ID. But in this case we
> > might need a way similar to the existing one to define the format.
> > Or maybe you should just resort to a UUID that is sent only when
> > some option is used ?
> 
> Yes, that would be my escape hatch if I could not get a stream. In fact
> the current version of my code has the unique ID on the connection.
> However I liked the idea of not adding another configuration option.

Yes it's always better if this can be avoided.

> Asking differently: If I wanted to upstream my proposal (with the
> explicit focus on unique IDs for TCP), how would you like to see it work?

I don't know yet. I'm just a bit concerned about the risk of having
something in TCP that disappears in HTTP if you do it for TCP only.
Someone using this connection ID for logging (that's often the purpose)
would only have it with TCP and would have to proceed differently for
HTTP. So I'm still wondering if it wouldn't be better to rely on
"proxy-v2-options" and decide what to send, and when the unique-id is
specified there, then it's taken from the stream, regardless of the
stream mode (tcp/http). Even if I consider that it doesn't make sense
to have this ID sent both on the connection and in a header, if there
is a use case to having such an ID on a connection, we must still be
sure that it doesn't become impossible to send it when in HTTP.

Also maybe having a way to retrieve a received ID on a listener as a
sample fetch so as to be able to build a unique-id header from it
could be useful. Some users want to pass the same ID along a whole
chain, and with this it would become possible at the TCP level, which
is quite nice.

Cheers,
Willy