subject:"proxy"

Alexander,

I now merged your patch with the SMP_VAL_ change, after verifying that
the reg-test is still OK. Thus 2.9-dev9 will contain it. Thanks!

Willy

Re: [PATCH 2/4] MEDIUM: connection: Send out generically allocated proxy-v2-options

On Fri, Nov 03, 2023 at 08:35:08PM +, Stephan, Alexander wrote:
> Hi Willy,
> 
> Thanks for the review. No problem for calling me Stephan, I am totally used
> to that, my teachers did that for years.

Oh I was sure you were used to but anyway I don't like calling people
incorrectly.

> > Yeah I agree, that part is not pretty. And as often in such code, not
> > handling errors makes them even more complicated to unroll as you
> > experienced in the list loop you added, which could otherwise have been
> > addressed using just a goto and a few free().
> 
> > Do not hesitate to propose extra patches to improve this, contributions
> > that make the code more reliable and maintainable are totally welcome!
> 
> Sure, if I have some extra time on my hands. Actually, a colleague of mine is
> preparing some patches like this. Since there are many small changes (e.g.,
> adding a free), should can we batch them or are individual commits required?

It depends. The rule of thumb for this is to think about the risk of breakage
and the ability to bisect (and possibly revert) later. But it's clear that
some API changes need to be performed atomically and can sometimes be a bit
larger than initially expected. Among the things we practice if some large
area changes are needed, is to try to make a first set of preparation patches
that apply most of the visual changes without changing semantics (e.g. move
some code into a separate function, or reindent 500 lines at once in an
"if (1)" block etc). And only then the patches that perform the real changes
are applied. This has the benefit that the first patch promises not to
change the logic so that it cannot break on bisect and doesn't need to be
analyzed later, and the patches that change the logic are much smaller.

> > Are you sure it was not a side effect of a debugging session with some 
> > temporary code in it ?
> 
> Pretty sure, yes. I'll will get back to this at the beginning of next week,
> if it's okay. I compiled with a debug flag, but I wouldn't usually expect
> this to be an issue.

OK let's try to figure that later. I'll merge your code in its current
form for now.

> > So if that's OK for you I can change it now before merging.
> 
> Ah, I had used a SMP_VAL_* before, but I was not 100% about the meaning. Then
> I fell back to the proxy. Feel free to change it!

;-)  Will do.

> > Yes very likely. Originally the code didn't check for allocation errors
> > during parsing because it was the boot phase, and we used to consider that
> > a memory allocation error would not always be reportable anyway, and given
> > that it was at boot time, the result was going to be the same.
> > But much later the code began to be a bit more dynamic, and such practices
> > are no longer acceptable in parts that can be called at run time (e.g.
> > during an "add server" on the CLI). It's particularly difficult to address
> > some of these historic remains but from time to time we manage to pass an
> > extra pointer to some functions to write an error, and make a function
> > return a set of flags among ERR_{WARN,FATAL,ALERT,ABORT}. But it takes a
> > lot of time > for little to no perceived value for the vast majority of
> > users :-(
> 
> Interesting how that came to be, no accusations. I see that this is not a
> high priority. Not sure if I have the time to provide a fix here but I'll
> keep it mind.

OK. No rush but similarly, patches welcome of course.

> > Another point, in make_proxy_line_v2() I'm seeing that you've placed
> > everything under "if (strm)". I understand why, it's because you didn't
> > want to call build_logline() without a stream. That made me think "hmmm we
> > already have the ability to do that", and I found it, you can use
> > sess_build_logline() instead, that takes the session separately, and
> > supports an optional stream. That would be useful for health checks for
> > example, since they don't have streams. But there's a catch: in function
> > make_proxy_line_v2() we don't have the session at the moment, and
> > build_logline() extracts it from the stream. Normally during a session
> > establishment, the session is present in connection->owner, thus
> > remote->owner in make_proxy_line_v2(). I suggest that you give this a
> > try to confirm that it works for you, this way we'd be sure that health
> > checks can also send the ppv2 fields. But that's not critical given that
> > there's no bad behavior right now, it's just that fields will be ignored,
> > so this can be done in a subsequent patch.
> 
> Again, I don't mind if you make the SMP_VAL_*.

OK will do it.

> I tried the
> sess_build_logline() and it seems to work perfectly. The tests also still
> pass, so looks good. I would like to provide a second patch for this next
> week, if this okay.

Sure, that would be great!

thanks and have a nice week-end!
Willy

RE: [PATCH 2/4] MEDIUM: connection: Send out generically allocated proxy-v2-options

2023-11-03 Thread Stephan, Alexander

Hi Willy,

Thanks for the review. No problem for calling me Stephan, I am totally used to 
that, my teachers did that for years.

> At first glance the patches look nice.
Great! 

> Yeah I agree, that part is not pretty. And as often in such code, not 
> handling errors makes them even more complicated to unroll as you experienced 
> in the list loop you added, which could otherwise have been addressed using 
> just a goto and a few free().

> Do not hesitate to propose extra patches to improve this, contributions that 
> make the code more reliable and maintainable are totally welcome!

Sure, if I have some extra time on my hands. Actually, a colleague of mine is 
preparing some patches like this. Since there are many small changes (e.g., 
adding a free), should can we batch them or are individual commits required?

> Are you sure it was not a side effect of a debugging session with some 
> temporary code in it ?

Pretty sure, yes. I'll will get back to this at the beginning of next week, if 
it's okay. I compiled with a debug flag, but I wouldn't usually expect this to 
be an issue.

> So if that's OK for you I can change it now before merging.

Ah, I had used a SMP_VAL_* before, but I was not 100% about the meaning. Then I 
fell back to the proxy. Feel free to change it!

> Yes very likely. Originally the code didn't check for allocation errors 
> during parsing because it was the boot phase, and we used to consider that a 
> memory allocation error would not always be reportable anyway, and given that 
> it was at boot time, the result was going to be the same.
> But much later the code began to be a bit more dynamic, and such practices 
> are no longer acceptable in parts that can be called at run time (e.g.
> during an "add server" on the CLI). It's particularly difficult to address 
> some of these historic remains but from time to time we manage to pass an 
> extra pointer to some functions to write an error, and make a function return 
> a set of flags among ERR_{WARN,FATAL,ALERT,ABORT}. But it takes a lot of time 
> > for little to no perceived value for the vast majority of users :-(

Interesting how that came to be, no accusations. I see that this is not a high 
priority. Not sure if I have the time to provide a fix here but I'll keep it 
mind.

> Another point, in make_proxy_line_v2() I'm seeing that you've placed 
> everything under "if (strm)". I understand why, it's because you didn't want 
> to call build_logline() without a stream. That made me think "hmmm we already 
> have the ability to do that", and I found it, you can use
> sess_build_logline() instead, that takes the session separately, and supports 
> an optional stream. That would be useful for health checks for example, since 
> they don't have streams. But there's a catch: in function
> make_proxy_line_v2() we don't have the session at the moment, and
> build_logline() extracts it from the stream. Normally during a session 
> establishment, the session is present in connection->owner, thus
> remote->owner in make_proxy_line_v2(). I suggest that you give this a
> try to confirm that it works for you, this way we'd be sure that health 
> checks can also send the ppv2 fields. But that's not critical given that 
> there's no bad behavior right now, it's just that fields will be ignored, so 
> this can be done in a subsequent patch.

Again, I don't mind if you make the SMP_VAL_*.  I tried the 
sess_build_logline() and it seems to work perfectly. The tests also still pass, 
so looks good. I would like to provide a second patch for this next week, if 
this okay.

Best,
Alexander

-Original Message-
From: Willy Tarreau  
Sent: Friday, November 3, 2023 8:11 PM
To: Stephan, Alexander 
Cc: haproxy@formilux.org
Subject: Re: [PATCH 2/4] MEDIUM: connection: Send out generically allocated 
proxy-v2-options

Hi Alexander,

(and BTW sorry for having called you Stephan twice in the last thread, each 
time I have to make a mental effort due to how your first and last names are 
presented in your e-mail address).

On Sat, Oct 28, 2023 at 07:32:20PM +, Stephan, Alexander wrote:
> I've just finished the implementation based on the tip with the 
> deferred log format parsing so the default-server should be also fully 
> supported now. ?

At first glance the patches look nice.

> I guess using the finalize method of the parser is the canonic implementation 
> of this.
> 
> I have a few remarks about the current state of the code:
> - srv_settings_cpy in server.c has no form of error handling 
> available. This is potentially causes trouble due to lack of error handling:
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2Fhaproxy%2Fhaproxy%2Fblob%2F47ed1181f29a56ccb04e5b80ef4f941446
> 6ada7a%2Fsrc%2Fserver.c%23L2

Re: [PATCH 2/4] MEDIUM: connection: Send out generically allocated proxy-v2-options

Hi Alexander,

(and BTW sorry for having called you Stephan twice in the last thread,
each time I have to make a mental effort due to how your first and last
names are presented in your e-mail address).

On Sat, Oct 28, 2023 at 07:32:20PM +, Stephan, Alexander wrote:
> I've just finished the implementation based on the tip with the deferred log
> format parsing so the default-server should be also fully supported now. ?

At first glance the patches look nice.

> I guess using the finalize method of the parser is the canonic implementation 
> of this.
> 
> I have a few remarks about the current state of the code:
> - srv_settings_cpy in server.c has no form of error handling available. This
> is potentially causes trouble due to lack of error handling:
> https://github.com/haproxy/haproxy/blob/47ed1181f29a56ccb04e5b80ef4f9414466ada7a/src/server.c#L2370
> For now, I did not want to change the signature, but I think, error handling
> would be beneficial here.

Yeah I agree, that part is not pretty. And as often in such code, not
handling errors makes them even more complicated to unroll as you
experienced in the list loop you added, which could otherwise have been
addressed using just a goto and a few free().

Do not hesitate to propose extra patches to improve this, contributions
that make the code more reliable and maintainable are totally welcome!

> - In the new list_for_each in srv_settings_cpy I have to check for srv_tlv ==
> NULL as for some reason, the list contains NULL entries. I don't see any
> mistakes with the list handling. Maybe it is just due to the structure of the
> server logic. 

I don't see how this is possible:

list_for_each_entry(srv_tlv, >pp_tlvs, list) {
if (srv_tlv == NULL)
break;

For srv_tlv to be NULL, it would mean that you visited (NULL)->list
at some point, and apparently pp_tlvs is always manipulated with
LIST_APPEND() only, so I don't see how you could end up with
something like last->next = (NULL)->list. Are you sure it was not a 
side effect of a debugging session with some temporary code in it ?
I'd be interested in knowing if you can reproduce it so that we can
find the root cause (and hopefully address it).

> - Please double check that my arguments for the parse_logformat_string
> function are correct. I omit log options for now and use the capabilities of
> the proxy. Seems like the best fit, but I could be wrong.

Ah good point. The argument you're missing and for which use used
px->cap is one of SMP_VAL_*. It indicates at which point(s) the
log-format may be called so that the parser can emit suitable
warnings if some sample fetch functions are not available. Here it
will be used during the server connect() phase, so there is already
one perfect for this, which is SMP_VAL_BE_SRV_CON. It's already used 
by the source and sni arguments. For the options I think it's OK that
it stays zero, that's how it's used everywhere else :-)

I could verify that your reg test still works with this change:

  --- a/src/server.c
  +++ b/src/server.c
  @@ -3252,7 +3252,7 @@ static int _srv_parse_finalize(char **args, int cur_arg,
  list_for_each_entry(srv_tlv, >pp_tlvs, list) {
  LIST_INIT(_tlv->fmt);
  if (srv_tlv->fmt_string && 
unlikely(!parse_logformat_string(srv_tlv->fmt_string,
  -   srv->proxy, _tlv->fmt, 0, px->cap, ))) {
  +   srv->proxy, _tlv->fmt, 0, SMP_VAL_BE_SRV_CON, 
))) {
  if (errmsg) {
  ha_alert("%s\n", errmsg);
  free(errmsg);

So if that's OK for you I can change it now before merging.

> - I noticed that there are also no checks for strdup in server.c, that might
> need to be addressed in the future. For the srv_settings_cpy I cannot give an
> error, but only try to avoid the memory leak.

Yes very likely. Originally the code didn't check for allocation errors
during parsing because it was the boot phase, and we used to consider
that a memory allocation error would not always be reportable anyway,
and given that it was at boot time, the result was going to be the same.
But much later the code began to be a bit more dynamic, and such practices
are no longer acceptable in parts that can be called at run time (e.g.
during an "add server" on the CLI). It's particularly difficult to
address some of these historic remains but from time to time we manage
to pass an extra pointer to some functions to write an error, and make a
function return a set of flags among ERR_{WARN,FATAL,ALERT,ABORT}. But
it takes a lot of time for little to no perceived value for the vast
majority of users :-(

Another point, in make_proxy_line_v2() I'm seeing that you've placed
everything under "if (strm)". I understand why, it's

Re: [PATCH 2/4] MEDIUM: connection: Send out generically allocated proxy-v2-options

On Fri, Nov 03, 2023 at 05:15:03PM +, Stephan, Alexander wrote:
> Hi Willy,
> 
> Sorry, my email client probably did something weird...
> I attached them now, should hopefully prevent any reformatting.

Thanks for the fast response. I'll check them keeping in mind your
last comments in your previous e-mail for patch2. Hopefully this
evening before issuing 2.9-dev9 with them.

Willy

RE: [PATCH 2/4] MEDIUM: connection: Send out generically allocated proxy-v2-options

2023-11-03 Thread Stephan, Alexander

Hi Willy,

Sorry, my email client probably did something weird...
I attached them now, should hopefully prevent any reformatting.

Best,
Alexander

-Original Message-
From: Willy Tarreau  
Sent: Friday, November 3, 2023 5:52 PM
To: Stephan, Alexander 
Cc: haproxy@formilux.org
Subject: Re: [PATCH 2/4] MEDIUM: connection: Send out generically allocated 
proxy-v2-options

On Fri, Nov 03, 2023 at 05:14:33PM +0100, Willy Tarreau wrote:
> Hi Stephan,
> 
> On Fri, Nov 03, 2023 at 01:54:26PM +, Stephan, Alexander wrote:
> > Hi Willy,
> > 
> > Did you receive the other two mails with the updated patches? I 
> > couldn't find it the reply to first page in the archive although I 
> > CCed the list. That's why I wanted to double-check, not to run in the 
> > previous situation.
> 
> Yes, sorry, it's just that I have been busy on other stuff and didn't 
> have the time to have a look yet, but I'm seeing them still marked 
> unread here. I'm doing my best to review and merge them ASAP.

Stephan, I just noticed that your patches were mangled with tabs turned to 
spaces. Please just resend them attached (the two in the same message if you 
want).

Thanks,
Willy


0001-MINOR-server-Add-parser-support-for-set-proxy-v2-tlv.patch
Description:  0001-MINOR-server-Add-parser-support-for-set-proxy-v2-tlv.patch


0002-MINOR-connection-Send-out-generic-user-defined-serve.patch
Description:  0002-MINOR-connection-Send-out-generic-user-defined-serve.patch

Re: [PATCH 2/4] MEDIUM: connection: Send out generically allocated proxy-v2-options

On Fri, Nov 03, 2023 at 05:14:33PM +0100, Willy Tarreau wrote:
> Hi Stephan,
> 
> On Fri, Nov 03, 2023 at 01:54:26PM +, Stephan, Alexander wrote:
> > Hi Willy,
> > 
> > Did you receive the other two mails with the updated patches? I couldn't 
> > find
> > it the reply to first page in the archive although I CCed the list. That's
> > why I wanted to double-check, not to run in the previous situation.
> 
> Yes, sorry, it's just that I have been busy on other stuff and didn't
> have the time to have a look yet, but I'm seeing them still marked
> unread here. I'm doing my best to review and merge them ASAP.

Stephan, I just noticed that your patches were mangled with tabs turned
to spaces. Please just resend them attached (the two in the same message
if you want).

Thanks,
Willy

Re: [PATCH 2/4] MEDIUM: connection: Send out generically allocated proxy-v2-options

Hi Stephan,

On Fri, Nov 03, 2023 at 01:54:26PM +, Stephan, Alexander wrote:
> Hi Willy,
> 
> Did you receive the other two mails with the updated patches? I couldn't find
> it the reply to first page in the archive although I CCed the list. That's
> why I wanted to double-check, not to run in the previous situation.

Yes, sorry, it's just that I have been busy on other stuff and didn't
have the time to have a look yet, but I'm seeing them still marked
unread here. I'm doing my best to review and merge them ASAP.

Thanks,
Willy

RE: [PATCH 2/4] MEDIUM: connection: Send out generically allocated proxy-v2-options

2023-11-03 Thread Stephan, Alexander

Hi Willy,

Did you receive the other two mails with the updated patches? I couldn't find 
it the reply to first page in the archive although I CCed the list. That's why 
I wanted to double-check, not to run in the previous situation.

Best,
Alexander

-Original Message-
From: Willy Tarreau  
Sent: Friday, October 27, 2023 4:22 PM
To: Stephan, Alexander 
Cc: haproxy@formilux.org
Subject: Re: [PATCH 2/4] MEDIUM: connection: Send out generically allocated 
proxy-v2-options

Hi Alexander,

On Fri, Oct 27, 2023 at 02:12:10PM +, Stephan, Alexander wrote:
> > BTW, please check if this works in default-server directives.
> 
> struct srv_pp_tlv_list {
> struct list list;
> struct list fmt;
> unsigned char type;
> };
> 
> To allow for use with the default server, I adjusted srv_settings_cpy 
> accordingly such that the server TLV entries (see above) are deep copied.
> It works, but there is an edge case with the struct logformat_node 
> that is contained in such a TLV struct. default-server directives Its 
> member expr has the type of a void pointer, therefore I cannot 
> directly copy it. Alternatively, if I would reuse the memory by simply 
> copying the pointer, a double free will likely happen in srv_drop (I 
> always free the TLV list in it, alongside the logformat node list).
> Besides, the servers created from the default-backend should operate 
> independently, so this is not an option, I guess.

Definitely. From what I remember about what we do for other such expressions 
that are inherited from defaults, we use a trick: we only save the expression 
as a string during parsing, that's the same that is kept on the default server. 
Thus the new server just does an strdup() of that log-format expression that's 
just a string. And only later we resolve it. That's not the best example but 
the first I just found, please have a look at uniqueid_format_string. It's 
handled exactly like this and resolved in check_config_validity().

> > You may want to have a look at how the "sni" keyword which also 
> > supports an expression is handled, as I don't recall the exact details.
> > Maybe in your case we don't need the expression but the log-format 
> > instead and it's not an issue, I just don't remember the details 
> > without having a deeper look to be honest.
> 
> Maybe let's just use a sample expression as its usage is more straight 
> forward, basically similar to the sni option? Or do you have any other 
> ideas on how to tackle the issue I described above?

Parsing the log-format string late definitely is the best option, and not too 
cumbersome. You can even still report the line number etc from the "server" 
struct to indicate in the parsing error if any, since everything is on the same 
line, so there's really no problem with parsing late.

> Disabling the default-server support could be another solution. I am 
> very interested in your opinion on this.

I'd really avoid disabling default-server as much as possible, or it will start 
to be quite difficult to configure given that other related options are 
accepted there. I tend to consider that the effort needed by users to work 
around our limited designs often outweighs the efforts we should have involved 
to make it smoother for them, so until we're certain it's not possible it's 
worth trying ;-)

Thanks!
Willy

RE: [PATCH 2/4] MEDIUM: connection: Send out generically allocated proxy-v2-options

2023-10-28 Thread Stephan, Alexander

Hi Willy,

I've just finished the implementation based on the tip with the deferred log 
format parsing so the default-server should be also fully supported now. 
I guess using the finalize method of the parser is the canonic implementation 
of this.

I have a few remarks about the current state of the code:
- srv_settings_cpy in server.c has no form of error handling available. This is 
potentially causes trouble due to lack of error handling: 
https://github.com/haproxy/haproxy/blob/47ed1181f29a56ccb04e5b80ef4f9414466ada7a/src/server.c#L2370
 For now, I did not want to change the signature, but I think, error handling 
would be beneficial here.
- In the new list_for_each in srv_settings_cpy I have to check for srv_tlv == 
NULL as for some reason, the list contains NULL entries. I don't see any 
mistakes with the list handling. Maybe it is just due to the structure of the 
server logic. 
- Please double check that my arguments for the parse_logformat_string function 
are correct. I omit log options for now and use the capabilities of the proxy. 
Seems like the best fit, but I could be wrong.
- I noticed that there are also no checks for strdup in server.c, that might 
need to be addressed in the future. For the srv_settings_cpy I cannot give an 
error, but only try to avoid the memory leak.

The successor of original patch for our replies is down below. I will send the 
other updated path (now [PATCH 1/2]) to the corresponding submitted mail. 
Again, please ignore [PATH 3/4] and [PATH 4/4] now.

Thanks and best,
Alexander
---
From 0f8691072ef17ceac98c6fffb063fdacc016a99c Mon Sep 17 00:00:00 2001
From: Alexander Stephan 
Date: Sat, 28 Oct 2023 20:57:07 +0200
Subject: [PATCH 2/2] MINOR: connection: Send out generic, user-defined server
 TLVs

To follow-up the implementation of the new set-proxy-v2-tlv-fmt
keyword in the server, the connection is updated to use the previously
allocated TLVs. If no value was specified, we send out an empty TLV.
As the feature is fully working with this commit, documentation and a
test for the server and default-server are added as well.
---
 doc/configuration.txt | 16 
 .../proxy_protocol_send_generic.vtc   | 74 +++
 src/connection.c  | 36 -
 3 files changed, 124 insertions(+), 2 deletions(-)
 create mode 100644 reg-tests/connection/proxy_protocol_send_generic.vtc

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 3dcdb3e2a..b54e9cd2a 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -16725,6 +16725,22 @@ send-proxy-v2
   of this version of the protocol. See also the "no-send-proxy-v2" option of
   this section and send-proxy" option of the "bind" keyword.

+set-proxy-v2-tlv-fmt() 
+  The "set-proxy-v2-tlv-fmt" parameter is used to send arbitrary PROXY protocol
+  version 2 TLVs. For the type () range of the defined TLV type please 
refer
+  to section 2.2.8. of the proxy protocol specification. However, the value can
+  be chosen freely as long as it does not exceed the maximum length of 65,535
+  bytes. It can also be used for forwarding TLVs by using the fetch "fc_pp_tlv"
+  to retrieve a received TLV from the frontend. It may be used as a server or
+  a default-server option. It must be used in combination with send-proxy-v2
+  such that PPv2 TLVs are actually sent out.
+
+  Example:
+  server srv1 192.168.1.1:80 send-proxy-v2 set-proxy-v2-tlv-fmt(0x20) 
%[fc_pp_tlv(0x20)]
+
+  In this case, we fetch the TLV with the type 0x20 as a string and set as the 
value
+  of a newly created TLV that also has the type 0x20.
+
 proxy-v2-options [,]*
   The "proxy-v2-options" parameter add options to send in PROXY protocol
   version 2 when "send-proxy-v2" is used. Options available are:
diff --git a/reg-tests/connection/proxy_protocol_send_generic.vtc 
b/reg-tests/connection/proxy_protocol_send_generic.vtc
new file mode 100644
index 0..f61bcae73
--- /dev/null
+++ b/reg-tests/connection/proxy_protocol_send_generic.vtc
@@ -0,0 +1,74 @@
+varnishtest "Check that generic TLV IDs are sent properly"
+
+#REQUIRE_VERSION=2.2
+
+feature ignore_unknown_macro
+
+haproxy h1 -conf {
+defaults
+mode http
+log global
+
+timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    listen sender
+bind "fd@${feS}"
+    server example ${h1_feR_addr}:${h1_feR_port} send-proxy-v2 
set-proxy-v2-tlv-fmt(0xE1) %[str("foo")] set-proxy-v2-tlv-fmt(0xE2)
+
+listen receiver
+bind "fd@${feR}" accept-proxy
+
+# Check that the TLV value is set in the backend.
+http-request set-var(txn.custom_tlv_a) fc_pp_tlv(0xE1)
+http-after-response set-header proxy_custom_tlv_a 
%[var(txn.cu

Re: [PATCH 2/4] MEDIUM: connection: Send out generically allocated proxy-v2-options

2023-10-27 Thread Willy Tarreau

Hi Alexander,

On Fri, Oct 27, 2023 at 02:12:10PM +, Stephan, Alexander wrote:
> > BTW, please check if this works in default-server directives.
> 
> struct srv_pp_tlv_list {
> struct list list;
> struct list fmt;
> unsigned char type;
> };
> 
> To allow for use with the default server, I adjusted srv_settings_cpy
> accordingly such that the server TLV entries (see above) are deep copied.
> It works, but there is an edge case with the struct logformat_node that is
> contained in such a TLV struct. default-server directives
> Its member expr has the type of a void pointer, therefore I cannot directly
> copy it. Alternatively, if I would reuse the memory by simply copying the
> pointer, a double free will likely happen in srv_drop (I always free the TLV
> list in it, alongside the logformat node list).
> Besides, the servers created from the default-backend should operate
> independently, so this is not an option, I guess.

Definitely. From what I remember about what we do for other such
expressions that are inherited from defaults, we use a trick: we only
save the expression as a string during parsing, that's the same that
is kept on the default server. Thus the new server just does an strdup()
of that log-format expression that's just a string. And only later we
resolve it. That's not the best example but the first I just found,
please have a look at uniqueid_format_string. It's handled exactly
like this and resolved in check_config_validity().

> > You may want to have a look at how the "sni" keyword which also supports an
> > expression is handled, as I don't recall the exact details.
> > Maybe in your case we don't need the expression but the log-format instead
> > and it's not an issue, I just don't remember the details without having a
> > deeper look to be honest.
> 
> Maybe let's just use a sample expression as its usage is more straight
> forward, basically similar to the sni option? Or do you have any other ideas
> on how to tackle the issue I described above?

Parsing the log-format string late definitely is the best option, and not
too cumbersome. You can even still report the line number etc from the
"server" struct to indicate in the parsing error if any, since everything
is on the same line, so there's really no problem with parsing late.

> Disabling the default-server support could be another solution. I am very
> interested in your opinion on this.

I'd really avoid disabling default-server as much as possible, or it
will start to be quite difficult to configure given that other related
options are accepted there. I tend to consider that the effort needed
by users to work around our limited designs often outweighs the efforts
we should have involved to make it smoother for them, so until we're
certain it's not possible it's worth trying ;-)

Thanks!
Willy

RE: [PATCH 2/4] MEDIUM: connection: Send out generically allocated proxy-v2-options

2023-10-27 Thread Stephan, Alexander

Hi Willy,

Thanks for clarifying the connection modification situation, I think I now 
understood the problem and the current state.

> No it's not too late. We're just trying to avoid last-minute breaking 
> changes, such as the ones we usually tend to do late and to regret, either 
> because they don't integrate well and make all of us spend our time trying to 
> fix corner cases instead of cleaning up the rest, or because they break stuff 
> late and complicate testing. Such a change is tiny and harmless, I'd almost 
> accept it the week before the release (but please don't do that).

Okay, great. I am really trying to get it done today, but I have a couple 
questions:

> BTW, please check if this works in default-server directives.

struct srv_pp_tlv_list {
struct list list;
struct list fmt;
unsigned char type;
};

To allow for use with the default server, I adjusted srv_settings_cpy 
accordingly such that the server TLV entries (see above) are deep copied.
It works, but there is an edge case with the struct logformat_node that is 
contained in such a TLV struct. default-server directives
Its member expr has the type of a void pointer, therefore I cannot directly 
copy it.
Alternatively, if I would reuse the memory by simply copying the pointer, a 
double free will likely happen in srv_drop (I always free the TLV list in it, 
alongside the logformat node list).
Besides, the servers created from the default-backend should operate 
independently, so this is not an option, I guess.

> You may want to have a look at how the "sni" keyword which also supports an 
> expression is handled, as I don't recall the exact details.
> Maybe in your case we don't need the expression but the log-format instead 
> and it's not an issue, I just don't remember the details without having a 
> deeper look to be honest.

Maybe let's just use a sample expression as its usage is more straight forward, 
basically similar to the sni option? Or do you have any other ideas on how to 
tackle the issue I described above?
Disabling the default-server support could be another solution. I am very 
interested in your opinion on this.

Best,
Alexander

-Original Message-
From: Willy Tarreau 
Sent: Monday, October 23, 2023 2:33 PM
To: Stephan, Alexander 
Cc: haproxy@formilux.org
Subject: Re: [PATCH 2/4] MEDIUM: connection: Send out generically allocated 
proxy-v2-options

Hi Alexander,

On Mon, Oct 23, 2023 at 12:07:39PM +, Stephan, Alexander wrote:
> We can ignore the last two commits for now (LOW: connection: Add TLV
> update function and MEDIUM: tcp-act: Add new set-tlv TCP action for PPv2 
> TLVs).
> Based on the first two commits, I created a diff that would implement
> something like you requested (new parser and no use of remote).
> It should be fully working, so you could even try it out locally if you want.
> If you think it looks promising, I will adjust the tests and the docs,
> and update the series.

OK.

> Now to address your feedback in more detail:
>
> > I'm really having an issue with using "the remote frontend connection"
> > here. We've done this mistake in the past with the transparent mode that 
> > connects to the original destination address, that resulted in "set-dst"
> > having to be implemented, then being broken by multiplexed streams (e.g.
> > h2), then having to be internally emulated at various layers (connection, 
> > session, stream, transaction etc). Same for src. The solution is not 
> > durable at all, and as you noticed, you already had to implement the 
> > ability to modify these fields before passing them, hence reintroducing 
> > that class of trouble on a new area.
>
> I choose to use the remote connection since it is already done that
> way for other TLV types like SRV_PP_V2_AUTHORITY
> (https://github.com/haproxy/haproxy/commit/8a4ffa0aabf8509098f95ac78fa48b18f415c42c).
> It simply forwards what is found the remote connection, if there is
> something set. Similarly, PP2_TYPE_NETNS and SRV_PP_V2_SSL depend on remote.
> First, I did not like the method with an extra fetch since it creates
> potential overhead. However, I am likely a bit excessive here. I am
> not opposed to not using remote at all. I simply was not aware of the
> intricacies.

Oh rest assured I'm not criticizing your choice, it's common to start from what 
exists, I was merely explaining that what exists is the result of poor choices 
and that we'd rather not spread them further ;-)

> > Also what will be left in the logs after you modify the contents ? etc.
> You mean that the log does not match the content of the actual TLV?

I mean once modified via a rule, it's difficult to know if the log contains a 
modified field or an original one.

> It could happen, I suppose. But why doesn't this problem with
>

Re: [PATCH 2/4] MEDIUM: connection: Send out generically allocated proxy-v2-options

2023-10-23 Thread Willy Tarreau

Hi Alexander,

On Mon, Oct 23, 2023 at 12:07:39PM +, Stephan, Alexander wrote:
> We can ignore the last two commits for now (LOW: connection: Add TLV update
> function and MEDIUM: tcp-act: Add new set-tlv TCP action for PPv2 TLVs).
> Based on the first two commits, I created a diff that would implement
> something like you requested (new parser and no use of remote).
> It should be fully working, so you could even try it out locally if you want.
> If you think it looks promising, I will adjust the tests and the docs, and
> update the series.

OK.

> Now to address your feedback in more detail:
> 
> > I'm really having an issue with using "the remote frontend connection"
> > here. We've done this mistake in the past with the transparent mode that 
> > connects to the original destination address, that resulted in "set-dst"
> > having to be implemented, then being broken by multiplexed streams (e.g.
> > h2), then having to be internally emulated at various layers (connection, 
> > session, stream, transaction etc). Same for src. The solution is not 
> > durable at all, and as you noticed, you already had to implement the 
> > ability to modify these fields before passing them, hence reintroducing 
> > that class of trouble on a new area. 
> 
> I choose to use the remote connection since it is already done that way for
> other TLV types like SRV_PP_V2_AUTHORITY
> (https://github.com/haproxy/haproxy/commit/8a4ffa0aabf8509098f95ac78fa48b18f415c42c).
> It simply forwards what is found the remote connection, if there is something
> set. Similarly, PP2_TYPE_NETNS and SRV_PP_V2_SSL depend on remote.
> First, I did not like the method with an extra fetch since it creates
> potential overhead. However, I am likely a bit excessive here. I am not
> opposed to not using remote at all. I simply was not aware of the
> intricacies.

Oh rest assured I'm not criticizing your choice, it's common to start
from what exists, I was merely explaining that what exists is the result
of poor choices and that we'd rather not spread them further ;-)

> > Also what will be left in the logs after you modify the contents ? etc.
> You mean that the log does not match the content of the actual TLV?

I mean once modified via a rule, it's difficult to know if the log contains
a modified field or an original one.

> It could happen, I suppose. But why doesn't this problem with multiplexed
> connections apply to all the TCP actions? As far as I know they all alter
> fields of the connection directly. Doesn't really matter though, I don't plan
> to use it anymore.

We do not modifiy the connection anymore since 2.7 or so, the set-src and
set-dst caused us a lot of misery for the exact reason you mentioned. Now
this is transparently intercepted at various levels to act either on the
stream, session or connection depending where it's done, and potentially
it will extract the lower value, modify it and assign it to an upper layer.
Yeah, that's totally ugly, but required to preserve compatibility with what
we've done before. We'd definitely not want to have to do that for ppv2 and
completely free fields! As a general rule we now try hard *not* to modify
existing information and rather use expressions or variables wherever
possible because it allows anyone to adjust the contents as they see fit
without having to later add exceptions for certain corner cases.

> > Why not something like "set-proxy-v2-tlv"? Maybe we then could also leave 
> > out the v2.
> 
> I would propose to go with the set-proxy-v2-tlv-fmt as you suggested later. I
> would leave in the v2 as other server options that concern v2 also leave it
> in. I don't mind being more verbose here. Realistically, there are only 2-3 of
> these set-proxy-v2-tlv-fmt anyway. Expression could make sense at some later
> point in time.

That's what I thought as well, not that many fields there so no big deal.

> > That's also why I'm extremely careful about the config shortcomings that
> > can come with it, because I suspect it could become popular and we really
> > want to avoid new fall traps.
> 
> Sure, that is completely understandable.
> 
> > I've been wondering if we want to use a format string or raw binary data
> > here, because some options are binary and nothing indicates that this will
> > not continue in the future. However since fc_pp_tlv() returns a string, I
> > guess most of them will continue this way.
> 
> I also think type string is fine.

Great!

> > Also once this is merged, a new problem will emerge. There's still no 
> > registry for the PPv2 TLV types.
> 
> I guess this should be okay for most cases where it is just used internally.
> But yeah, most people will just come up with TLVs off the top their head

RE: [PATCH 2/4] MEDIUM: connection: Send out generically allocated proxy-v2-options

2023-10-23 Thread Stephan, Alexander

Hi Willy,

Thanks for your feedback once again.

TLDR: I agree with your proposed changes. So, here is my corresponding proposal:

We can ignore the last two commits for now (LOW: connection: Add TLV update 
function and MEDIUM: tcp-act: Add new set-tlv TCP action for PPv2 TLVs).
Based on the first two commits, I created a diff that would implement something 
like you requested (new parser and no use of remote).
It should be fully working, so you could even try it out locally if you want. 
If you think it looks promising, I will adjust the tests and the docs, and 
update the series.

Now to address your feedback in more detail:

> I'm really having an issue with using "the remote frontend connection"
> here. We've done this mistake in the past with the transparent mode that 
> connects to the original destination address, that resulted in "set-dst"
> having to be implemented, then being broken by multiplexed streams (e.g.
> h2), then having to be internally emulated at various layers (connection, 
> session, stream, transaction etc). Same for src. The solution is not durable 
> at all, and as you noticed, you already had to implement the ability to 
> modify these fields before passing them, hence reintroducing that class of 
> trouble on a new area. 

I choose to use the remote connection since it is already done that way for 
other TLV types like SRV_PP_V2_AUTHORITY 
(https://github.com/haproxy/haproxy/commit/8a4ffa0aabf8509098f95ac78fa48b18f415c42c).
It simply forwards what is found the remote connection, if there is something 
set. Similarly, PP2_TYPE_NETNS and SRV_PP_V2_SSL depend on remote.
First, I did not like the method with an extra fetch since it creates potential 
overhead. However, I am likely a bit excessive here. I am not opposed to not 
using remote at all. I simply was not aware of the intricacies.

> Also what will be left in the logs after you modify the contents ? etc.
You mean that the log does not match the content of the actual TLV? It could 
happen, I suppose. But why doesn't this problem with multiplexed connections 
apply to all the TCP actions? As far as I know they all alter fields of the 
connection directly. Doesn't really matter though, I don't plan to use it 
anymore.

> I'm seeing another issue with passing type=fmt like this. The option is 
> currently defined as cutting around commas, and that's what it does. This 
> means that the format will not be parsable (no sample fetch functions with 
> more than one argument, no converters). Example:
>
>   proxy-v2-options 0xEE=%[var(txn.clientname,_unknown_)]
>^ ^^^
>First option  Second option

I see, that is a problem. Of course, we could track whether we are currently in 
a log format expression. But I think I have come up with a better 
implementation, see below.

> Why not something like "set-proxy-v2-tlv"? Maybe we then could also leave out 
> the v2.

I would propose to go with the set-proxy-v2-tlv-fmt as you suggested later. I 
would leave in the v2 as other server options that concern v2 also leave it in.
I don't mind being more verbose here. Realistically, there are only 2-3 of 
these set-proxy-v2-tlv-fmt anyway. Expression could make sense at some later 
point in time.

> That's also why I'm extremely careful about the config shortcomings that can 
> come with it, because I suspect it could become popular and we really want to 
> avoid new fall traps.

Sure, that is completely understandable.

> I've been wondering if we want to use a format string or raw binary data 
> here, because some options are binary and nothing indicates that this will 
> not continue in the future. However since fc_pp_tlv() returns a string, I 
> guess most of them will continue this way.

I also think type string is fine.

> Also once this is merged, a new problem will emerge. There's still no 
> registry for the PPv2 TLV types.

I guess this should be okay for most cases where it is just used internally. 
But yeah, most people will just come up with TLVs off the top their head. I 
will definitely add a note about the ranges.

> Given that you had previously contributed the ability to fetch a TLV field 
> from the front connection's proxy protocol using fc_pp_tlv(), I'd rather make 
> use of these ones explicitly. If you need to change them, then you just 
> assign them to a variable and edit that variable, then send the variable. Or 
> you can change them in-flight using a converter. Of course it would make 
> configs slightly more verbose, but they would always do the right thing and 
> we would not have to continue to special-case them over time because they 
> break expectations like set-src/set-dst did.

Generally, I wouldn't mind a more verbose config. As usually there shouldn't be 
too many TLV types that ne

Re: Override X-Forwarded-Port with send-proxy-v2

2023-10-19 Thread kvaps

My bad, I was testing it on wrong instance.

tcp-request content set-dst-port int(80)

is working fine :-)


Best Regards,
Andrei Kvapil


On Thu, Oct 19, 2023 at 8:06 PM kvaps  wrote:

> Hi,
>
> I run haproxy in non-privileged container, so I can't bind on higher port
> eg 80 and 443
> Thus I binded it to 8080 and 8443.
>
> But kubernetes service listening on normal port: 80 and 443
>
> I use tcp mode with proxy protocol (send-proxy-v2) to preserve the
> client's real IP and port.
> On the backend server I see that requests are expanded to:
>
> X-Forwarded-Port: 8080 and X-Forwarded-Port: 8433
>
> How can I override them to 80 and 443 in haproxy configuration?
>
> I tried setting the following options to frontend section:
>
> http-request set-header X-Forwarded-Port int(80)
> tcp-request content set-dst-port int(80)
> tcp-request connection set-dst-port int(80)
>
> None of them didn't make an effect :(
>
> Best Regards,
> Andrei Kvapil
>

Override X-Forwarded-Port with send-proxy-v2

2023-10-19 Thread kvaps

Hi,

I run haproxy in non-privileged container, so I can't bind on higher port
eg 80 and 443
Thus I binded it to 8080 and 8443.

But kubernetes service listening on normal port: 80 and 443

I use tcp mode with proxy protocol (send-proxy-v2) to preserve the client's
real IP and port.
On the backend server I see that requests are expanded to:

X-Forwarded-Port: 8080 and X-Forwarded-Port: 8433

How can I override them to 80 and 443 in haproxy configuration?

I tried setting the following options to frontend section:

http-request set-header X-Forwarded-Port int(80)
tcp-request content set-dst-port int(80)
tcp-request connection set-dst-port int(80)

None of them didn't make an effect :(

Best Regards,
Andrei Kvapil

Re: [PATCH 2/4] MEDIUM: connection: Send out generically allocated proxy-v2-options

2023-10-18 Thread Willy Tarreau

Hi Alexander,

I'm starting from the doc as it eases the discussion.

On Thu, Oct 05, 2023 at 11:05:50AM +, Stephan, Alexander wrote:
> --- a/doc/configuration.txt
> +++ b/doc/configuration.txt
> @@ -16671,6 +16671,26 @@ proxy-v2-options [,]*
>  generated unique ID is also used for the first HTTP request
>  within a Keep-Alive connection.
> 
> +  Besides, you can specify the type of TLV numerically instead.
> +
> +  Example 1:
> +  server example 127.0.0.1:2319 send-proxy-v2 proxy-v2-options crc32c,0xEE
> +
> +  The following logic applies: By default, the remote frontend connection is
> +  searched for the value 0xEE. If found, it is simply forwarded. Otherwise,
> +  a TLV with the ID "0xEE" and empty payload is sent out. In addition, crc32c
> +  is also set here.
> +
> +  You can also specify a key-value pair = syntax.  represents 
> the
> +  8 bit TLV type field in the range of 0 to 255. It can be expressed in 
> decimal
> +  or hexadecimal format (prefixed by "0x").
> +
> +  Example 2:
> +  server example_server 127.0.0.1:2319 send-proxy-v2 proxy-v2-options 
> 0xEE=%[str("foo")]
> +
> +  This will always send out the value "foo". Another common use case would 
> be to
> +  reference a variable.

I'm really having an issue with using "the remote frontend connection"
here. We've done this mistake in the past with the transparent mode that
connects to the original destination address, that resulted in "set-dst"
having to be implemented, then being broken by multiplexed streams (e.g.
h2), then having to be internally emulated at various layers (connection,
session, stream, transaction etc). Same for src. The solution is not
durable at all, and as you noticed, you already had to implement the
ability to modify these fields before passing them, hence reintroducing
that class of trouble on a new area. Also what will be left in the logs
after you modify the contents ? etc.

Given that you had previously contributed the ability to fetch a TLV
field from the front connection's proxy protocol using fc_pp_tlv(),
I'd rather make use of these ones explicitly. If you need to change
them, then you just assign them to a variable and edit that variable,
then send the variable. Or you can change them in-flight using a
converter. Of course it would make configs slightly more verbose, but
they would always do the right thing and we would not have to continue
to special-case them over time because they break expectations like
set-src/set-dst did.

I'm seeing another issue with passing type=fmt like this. The option is
currently defined as cutting around commas, and that's what it does. This
means that the format will not be parsable (no sample fetch functions with
more than one argument, no converters). Example:

   proxy-v2-options 0xEE=%[var(txn.clientname,_unknown_)]
^ ^^^
First option  Second option

Also the syntax in the format something=value is not that great for config
parsers and generators. However it looks like our server keywords support
parenthesis, so I think that instead you could do the following:

   proxy-v2-options(0xEE) %[var(txn.clientname,_unknown_)]

In this case the proxy-v2-options keyword would only add one option and
take a log-format, and you'd add as many such keywords for each option.
Note that from a configuration perspective this is cleaner as it's easier
to maintain value pairs like this for config management. But I don't think
the option name is that great especially to send a single one. So maybe
this one instead should be "proxy-v2-option(type) " though it may
maintain some confusion (option vs options). The other solution might be
to have a shorter one, which is also nicer when using many of these, e.g.
"proxy-v2-opt()".

Such an approach would basically split the proxy-v2 usage in two groups:
  - options that are either there or not there (proxy-v2-options)
  - options that need to take a value (proxy-v2-opt()).

I've been wondering if we want to use a format string or raw binary data
here, because some options are binary and nothing indicates that this will
not continue in the future. However since fc_pp_tlv() returns a string, I
guess most of them will continue this way. For set-var() we're using a raw
typed expression, but then we created set-var-fmt() to have one that takes
a log format. Just thinking loud, but then what about "proxy-v2-optfmt()"
or even "proxy-v2-tlvfmt()" or "pp-tlv-fmt()" (after all when speaking
about tlv we imply v2, right?). That would allow us later to implement
"pp-tlv()" taking an expression if really needed, e.g. to pass a DER
certificate, a SHA256, a sub-tlv or any such a thing.

Generally speaking I'm fine with the approach of m

Re: HA Proxy

2023-10-13 Thread Aleksandar Lazic

Hi Mohammed.

Yes HAProxy supports all of the requested capacity and features from
below. For a nice example what HAProxy is able to handle can you read
this Blog post.
https://www.haproxy.com/blog/haproxy-forwards-over-2-million-http-requests-per-second-on-a-single-aws-arm-instance

The very detailed Documentation can be found in the Web
https://docs.haproxy.org/ or in the source repository under the doc
directory
https://git.haproxy.org/?p=haproxy.git;a=tree;f=doc;h=9a53977a683fd7e80f23fff2a18ef192ca908636;hb=HEAD

There are very good examples and explanations for HAProxy features on
the HAProxy com Blog page https://www.haproxy.com/blog and you can also
find some examples with your favorite Search engine. Please take care
that some search results refer to some previous HAPRoxy Versions which
are not maintained anymore, this means that the founded solution could
work or need some rework for the current versions.

HAProxy have two versions the Opensource one and the Enterprise one.

If your company want support and is willing to pay for that can you get
in touch with HAProxy Sales via the contact from
https://www.haproxy.com/contact-us for the HAProxy Enterprise version
https://www.haproxy.com/products/haproxy-enterprise.

Hth with best regards

Alex

On 2023-10-13 (Fr.) 09:41, Mohammed Anees A wrote:

Hi Team

We have a requirement to for a Software based NLB to Load Balance an
enterprise application.

Following are the Capacity and Features of NLB required. Please
confirm, does HA Proxy supports the below capacity and features ?. let
us know the licensing model and Support structure.

Capacity :

* Requests per Second = 5000 RPS
* Concurrent Connections = 5000 Concurrent Sessions.
* Throughput = 40 Mbps

Features :

1. *Routing Profile *

Routing profile can be TCP based (layer 4) or HTTP based (layer 7).

2. *Load Balancing Method*

All load balancing methods are supported. It is recommended to use
Least Connections or Round Robin load balancing methods, for better
distribution between Application servers.

3. *Session persistence (stickiness)*

The LB must be configured with session persistence to enable a session
connection with the same application server instance. Configure
session persistence in all levels of load balancing (for example, if
there is a global load balancer in front of a few local load balancers).

To achieve session persistence, configure the LB with one of the
following persistence profiles:

* HTTP Cookie
* Client IP (Source address)

4. *Health monitoring*

An important property of an LB is the ability to perform health
monitoring checks (heartbeats) on each Application server. By using
health monitors, the LB verifies the server response or checks for any
network problems that can prevent a client from reaching a server. By
doing so, the LB can place the server in or out of service and can
make reliable load-balancing and high availability decisions.**

A common and recommended health monitor is *HTTP GET Request.***

5. *Idle (execution) timeout*

Setting the execution timeout controls termination of idle
connections. Configure an execution timeout of at least 4 hours.

6. *HTTPS Configuration*

The load balancer supports several HTTPS configuration methods.

These include:

* SSL bridging
* SSL offload
* SSL pass-through

SSL bridging and SSL offload are supported in HTTP based routing
(layer 7), and require deploying TLS certificate on the LB. SSL
pass-through is supported in TCP based routing (layer 4), and does not
require deploying a certificate on the LB.

Regards

Mohammed Anees

+91 9944170656

HA Proxy

2023-10-13 Thread Mohammed Anees A

Hi Team



We have a requirement to for a Software based NLB to Load Balance an
enterprise application.



Following are the Capacity and Features of NLB required. Please confirm,
does HA Proxy supports the below capacity and features ?. let us know the
licensing model and Support structure.



Capacity :



   - Requests per Second =  5000 RPS
   - Concurrent Connections = 5000 Concurrent Sessions.
   - Throughput = 40 Mbps

Features :



   1. *Routing Profile *

Routing profile can be TCP based (layer 4) or HTTP based (layer 7).



   1. *Load Balancing Method*

All load balancing methods are supported. It is recommended to use Least
Connections or Round Robin load balancing methods, for better distribution
between Application servers.

   1. *Session persistence (stickiness)*

The LB must be configured with session persistence to enable a session
connection with the same application server instance. Configure session
persistence in all levels of load balancing (for example, if there is a
global load balancer in front of a few local load balancers).

To achieve session persistence, configure the LB with one of the following
persistence profiles:

   - HTTP Cookie
   - Client IP (Source address)



   1. *Health monitoring*



An important property of an LB is the ability to perform health monitoring
checks (heartbeats) on each Application server. By using health monitors,
the LB verifies the server response or checks for any network problems that
can prevent a client from reaching a server. By doing so, the LB can place
the server in or out of service and can make reliable load-balancing and
high availability decisions.

A common and recommended health monitor is *HTTP GET Request.*

   1. *Idle (execution) timeout*



Setting the execution timeout controls termination of idle connections.
Configure an execution timeout of at least 4 hours.





   1. *HTTPS Configuration*



The load balancer supports several HTTPS configuration methods.



These include:

   - SSL bridging
   - SSL offload
   - SSL pass-through



SSL bridging and SSL offload are supported in HTTP based routing (layer 7),
and require deploying TLS certificate on the LB. SSL pass-through is
supported in TCP based routing (layer 4), and does not require deploying a
certificate on the LB.





Regards

Mohammed Anees

+91 9944170656

RE: [PATCH 2/4] MEDIUM: connection: Send out generically allocated proxy-v2-options

2023-10-05 Thread Stephan, Alexander

From 84608ed754c1a92e85e03036e8b0cd0949721ffb Mon Sep 17 00:00:00 2001
From: Alexander Stephan 
mailto:alexander.step...@sap.com>>
Date: Fri, 15 Sep 2023 12:42:36 +0200
Subject: [PATCH 2/4] MEDIUM: connection: Send out generically allocated
 proxy-v2-options

This commit removes the previous limitations on the existing, fixed PPv2 TLV 
types
that can be sent out. This is done by leveraging the previously implemented
parsing. We iterate over the server TLV list and either forward or send a new
TLV, depending on whether a TLV exists in the remote connection.
---
 doc/configuration.txt | 20 
 .../proxy_protocol_send_generic.vtc   | 35 +
 src/connection.c  | 51 +--
 3 files changed, 103 insertions(+), 3 deletions(-)
 create mode 100644 reg-tests/connection/proxy_protocol_send_generic.vtc

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 14f6b906d..aeff9e4db 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -16671,6 +16671,26 @@ proxy-v2-options [,]*
 generated unique ID is also used for the first HTTP request
 within a Keep-Alive connection.

+  Besides, you can specify the type of TLV numerically instead.
+
+  Example 1:
+  server example 127.0.0.1:2319 send-proxy-v2 proxy-v2-options crc32c,0xEE
+
+  The following logic applies: By default, the remote frontend connection is
+  searched for the value 0xEE. If found, it is simply forwarded. Otherwise,
+  a TLV with the ID "0xEE" and empty payload is sent out. In addition, crc32c
+  is also set here.
+
+  You can also specify a key-value pair = syntax.  represents the
+  8 bit TLV type field in the range of 0 to 255. It can be expressed in decimal
+  or hexadecimal format (prefixed by "0x").
+
+  Example 2:
+  server example_server 127.0.0.1:2319 send-proxy-v2 proxy-v2-options 
0xEE=%[str("foo")]
+
+  This will always send out the value "foo". Another common use case would be 
to
+  reference a variable.
+
 send-proxy-v2-ssl
   The "send-proxy-v2-ssl" parameter enforces use of the PROXY protocol version
   2 over any connection established to this server. The PROXY protocol informs
diff --git a/reg-tests/connection/proxy_protocol_send_generic.vtc 
b/reg-tests/connection/proxy_protocol_send_generic.vtc
new file mode 100644
index 0..e0bd15a1b
--- /dev/null
+++ b/reg-tests/connection/proxy_protocol_send_generic.vtc
@@ -0,0 +1,35 @@
+varnishtest "Check that generic TLV IDs are sent properly"
+
+#REQUIRE_VERSION=2.2
+
+feature ignore_unknown_macro
+
+haproxy h1 -conf {
+defaults
+mode http
+log global
+
+listen sender
+bind "fd@${feS}"
+server example ${h1_feR_addr}:${h1_feR_port} send-proxy-v2 
proxy-v2-options 0xE1=%[str("foo")],0xE2
+
+listen receiver
+bind "fd@${feR}" accept-proxy
+
+# Check that the TLV value is set in the backend.
+http-request set-var(txn.custom_tlv_a) fc_pp_tlv(0xE1)
+http-after-response set-header proxy_custom_tlv_a 
%[var(txn.custom_tlv_a)]
+
+# Check that TLVs without an value are sent out.
+http-request set-var(txn.custom_tlv_b) fc_pp_tlv(0xE2)
+http-after-response set-header proxy_custom_tlv_b 
%[var(txn.custom_tlv_b)]
+
+http-request return status 200
+} -start
+
+client c1 -connect ${h1_feS_sock} {
+txreq -url "/"
+rxresp
+expect resp.http.proxy_custom_tlv_a == "foo"
+expect resp.http.proxy_custom_tlv_b == ""
+} -run
diff --git a/src/connection.c b/src/connection.c
index 5f7226aae..51584b1ed 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -548,7 +549,7 @@ struct connection *conn_new(void *target)
 /* Releases a connection previously allocated by conn_new() */
 void conn_free(struct connection *conn)
 {
-   struct conn_tlv_list *tlv, *tlv_back = NULL;
+   struct conn_tlv_list *tlv = NULL, *tlv_back = NULL;

if (conn_is_back(conn))
conn_backend_deinit(conn);
@@ -1920,10 +1921,12 @@ static int make_proxy_line_v2(char *buf, int buf_len, 
struct server *srv, struct
int ret = 0;
struct proxy_hdr_v2 *hdr = (struct proxy_hdr_v2 *)buf;
struct sockaddr_storage null_addr = { .ss_family = 0 };
+   struct srv_pp_tlv_list *srv_tlv = NULL;
const struct sockaddr_storage *src = _addr;
const struct sockaddr_storage *dst = _addr;
-   const char *value;
-   int value_len;
+   const char *value = "";
+   int value_len = 0;
+   int found = 0;

if (buf_len < PP2_HEADER_LEN)
return 0;
@@ -1993,6 +1996,48 @@ static int make_proxy_line_v2(char *buf, int buf_len, 
struct server *srv, struct
}
}

+   if (strm) {
+   struct buffer *replace = NULL;
+
+   list_

RE: [PATCH 1/4] MEDIUM: server: Parse generic type-value pairs as proxy-v2-options

2023-10-05 Thread Stephan, Alexander

From fb8714c5aebd7fe957264d0f2234182f55f952fe Mon Sep 17 00:00:00 2001
From: Alexander Stephan 
Date: Fri, 15 Sep 2023 12:38:46 +0200
Subject: [PATCH 1/4] MEDIUM: server: Parse generic type-value pairs as
proxy-v2-options

This commit introduces a generic server-side parsing of type-value pair
arguments and allocation of a TLV list. This allows to 1) forward any TLV
type, 2) generally send out any TLV type, and 3) allows to specify concrete
values via an '=' after the type and a log fmt expression. If there is no
TLV found in the remote connection, an empty TLV is sent out.
---
include/haproxy/server-t.h | 10 +
src/server.c   | 76 +++---
2 files changed, 81 insertions(+), 5 deletions(-)

diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index 1175a470e..7b93bf48f 100644
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -256,6 +256,15 @@ enum __attribute__((__packed__)) srv_ws_mode {
SRV_WS_H2,
};
+/* Server-side TLV list, contains the types of the TLVs that should be sent 
out.
+ * Additionally, it can contain data, if specified in the config.
+ */
+struct srv_pp_tlv_list {
+   struct list list;
+   struct list fmt;
+   unsigned char type;
+};
+
struct proxy;
struct server {
/* mostly config or admin stuff, doesn't change often */
@@ -421,6 +430,7 @@ struct server {
struct list srv_rec_item;   /* to attach server to a srv record item */
struct list ip_rec_item;/* to attach server to a A or  record 
item */
struct ebpt_node host_dn;   /* hostdn store for srvrq and state file 
matching*/
+   struct list pp_tlvs;/* to send out PROXY protocol v2 TLVs */
struct task *srvrq_check;   /* Task testing SRV record 
expiration date for this server */
struct {
const char *file;   /* file where the section appears */
diff --git a/src/server.c b/src/server.c
index 3673340d1..08f86d030 100644
--- a/src/server.c
+++ b/src/server.c
@@ -1036,7 +1036,13 @@ static int srv_parse_proto(char **args, int *cur_arg,
static int srv_parse_proxy_v2_options(char **args, int *cur_arg,
  struct proxy *px, struct server *newsrv, char **err)
{
-   char *p, *n;
+   char *p = NULL, *n = NULL, *m = NULL;
+   char *key = NULL, *val = NULL;
+   char *endp = NULL;
+   unsigned char idx = 0;
+   size_t key_length = 0, val_length = 0;
+   struct srv_pp_tlv_list *srv_tlv = NULL;
+
for (p = args[*cur_arg+1]; p; p = n) {
n = strchr(p, ',');
if (n)
@@ -1061,13 +1067,61 @@ static int srv_parse_proxy_v2_options(char **args, int 
*cur_arg,
newsrv->pp_opts |= SRV_PP_V2_CRC32C;
} else if (strcmp(p, "unique-id") == 0) {
newsrv->pp_opts |= SRV_PP_V2_UNIQUE_ID;
-   } else
-   goto fail;
+   } else {
+   /* reset val in case it was set before */
+   val = NULL;
+
+   /* try to split by '=' into generic pair key value pair 
(=) */
+   m = strchr(p, '=');
+   if (m) {
+   key_length = m - p;
+   key = (char *) malloc(key_length + 1);
+   if (unlikely(!key))
+   goto fail;
+   snprintf(key, key_length + 1, "%s", p);
+
+   val_length = strlen(p) - key_length;
+   val = (char *) malloc(val_length + 1);
+   if (unlikely(!val))
+   goto fail;
+   snprintf(val, val_length + 1, "%s", m + 1);
+   }
+   else {
+   key = (char *) malloc(strlen(p) + 1);
+   if (unlikely(!key))
+   goto fail;
+   snprintf(key, strlen(p) + 1, "%s", p);
+   }
+
+   errno = 0;
+   idx = strtoul(key, , 0);
+   if (unlikely((endp && *endp) != '\0' || (key == endp) || (errno != 
0)))
+   goto fail;
+
+   /* processing is done in connection.c */
+   srv_tlv = malloc(sizeof(struct srv_pp_tlv_list));
+   if (unlikely(!srv_tlv))
+   goto fail;
+
+   srv_tlv->type = idx;
+   LIST_INIT(_tlv->fmt);
+   if (val && unlikely(!parse_logformat_string(val, px, _tlv->fmt, 
LOG_OPT_HTTP, PR_CAP_LISTEN, err)))
+   goto fail;
+
+   LIST_APPEND(>pp_tlvs, _tlv->list);
+   free(key);
+   free(val);
+   }
}
return 0;
- fail:
+fail:
+   free(key);
+   free(val);
+   free(srv_tlv);
+   errno = 0;
+
if (err)
-   memprintf(err, "'%s' : proxy v2 option not implemented", p);
+   memprintf(err, "'%s' : failed to set proxy v2 option", p);
    return ERR_ALERT | ERR_FATAL;
}
@@ -2428,6 +2482,7 @@ struct server *new_server(struct proxy *proxy)
LIST_APPEND(_list, >global_list);
LIST_INIT(>srv_rec_item);
LIST_INIT(>ip_rec_item);
+   LIST_INIT(>pp_tlvs);

RE: [PATCH 2/4] MEDIUM: connection: Send out generically allocated proxy-v2-options

2023-09-15 Thread Stephan, Alexander

From 84608ed754c1a92e85e03036e8b0cd0949721ffb Mon Sep 17 00:00:00 2001
From: Alexander Stephan 
Date: Fri, 15 Sep 2023 12:42:36 +0200
Subject: [PATCH 2/4] MEDIUM: connection: Send out generically allocated
 proxy-v2-options

This commit removes the previous limitations on the existing, fixed PPv2 TLV 
types
that can be sent out. This is done by leveraging the previously implemented
parsing. We iterate over the server TLV list and either forward or send a new
TLV, depending on whether a TLV exists in the remote connection.
---
 doc/configuration.txt | 20 
 .../proxy_protocol_send_generic.vtc   | 35 +
 src/connection.c  | 51 +--
 3 files changed, 103 insertions(+), 3 deletions(-)
 create mode 100644 reg-tests/connection/proxy_protocol_send_generic.vtc

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 14f6b906d..aeff9e4db 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -16671,6 +16671,26 @@ proxy-v2-options [,]*
 generated unique ID is also used for the first HTTP request
 within a Keep-Alive connection.

+  Besides, you can specify the type of TLV numerically instead.
+
+  Example 1:
+  server example 127.0.0.1:2319 send-proxy-v2 proxy-v2-options crc32c,0xEE
+
+  The following logic applies: By default, the remote frontend connection is
+  searched for the value 0xEE. If found, it is simply forwarded. Otherwise,
+  a TLV with the ID "0xEE" and empty payload is sent out. In addition, crc32c
+  is also set here.
+
+  You can also specify a key-value pair = syntax.  represents the
+  8 bit TLV type field in the range of 0 to 255. It can be expressed in decimal
+  or hexadecimal format (prefixed by "0x").
+
+  Example 2:
+  server example_server 127.0.0.1:2319 send-proxy-v2 proxy-v2-options 
0xEE=%[str("foo")]
+
+  This will always send out the value "foo". Another common use case would be 
to
+  reference a variable.
+
 send-proxy-v2-ssl
   The "send-proxy-v2-ssl" parameter enforces use of the PROXY protocol version
   2 over any connection established to this server. The PROXY protocol informs
diff --git a/reg-tests/connection/proxy_protocol_send_generic.vtc 
b/reg-tests/connection/proxy_protocol_send_generic.vtc
new file mode 100644
index 0..e0bd15a1b
--- /dev/null
+++ b/reg-tests/connection/proxy_protocol_send_generic.vtc
@@ -0,0 +1,35 @@
+varnishtest "Check that generic TLV IDs are sent properly"
+
+#REQUIRE_VERSION=2.2
+
+feature ignore_unknown_macro
+
+haproxy h1 -conf {
+defaults
+mode http
+log global
+
+listen sender
+bind "fd@${feS}"
+    server example ${h1_feR_addr}:${h1_feR_port} send-proxy-v2 
proxy-v2-options 0xE1=%[str("foo")],0xE2
+
+listen receiver
+bind "fd@${feR}" accept-proxy
+
+# Check that the TLV value is set in the backend.
+http-request set-var(txn.custom_tlv_a) fc_pp_tlv(0xE1)
+http-after-response set-header proxy_custom_tlv_a 
%[var(txn.custom_tlv_a)]
+
+# Check that TLVs without an value are sent out.
+http-request set-var(txn.custom_tlv_b) fc_pp_tlv(0xE2)
+http-after-response set-header proxy_custom_tlv_b 
%[var(txn.custom_tlv_b)]
+
+http-request return status 200
+} -start
+
+client c1 -connect ${h1_feS_sock} {
+txreq -url "/"
+rxresp
+expect resp.http.proxy_custom_tlv_a == "foo"
+expect resp.http.proxy_custom_tlv_b == ""
+} -run
diff --git a/src/connection.c b/src/connection.c
index 5f7226aae..51584b1ed 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -548,7 +549,7 @@ struct connection *conn_new(void *target)
 /* Releases a connection previously allocated by conn_new() */
 void conn_free(struct connection *conn)
 {
-   struct conn_tlv_list *tlv, *tlv_back = NULL;
+   struct conn_tlv_list *tlv = NULL, *tlv_back = NULL;

if (conn_is_back(conn))
conn_backend_deinit(conn);
@@ -1920,10 +1921,12 @@ static int make_proxy_line_v2(char *buf, int buf_len, 
struct server *srv, struct
int ret = 0;
struct proxy_hdr_v2 *hdr = (struct proxy_hdr_v2 *)buf;
struct sockaddr_storage null_addr = { .ss_family = 0 };
+   struct srv_pp_tlv_list *srv_tlv = NULL;
const struct sockaddr_storage *src = _addr;
const struct sockaddr_storage *dst = _addr;
-   const char *value;
-   int value_len;
+   const char *value = "";
+   int value_len = 0;
+   int found = 0;

if (buf_len < PP2_HEADER_LEN)
return 0;
@@ -1993,6 +1996,48 @@ static int make_proxy_line_v2(char *buf, int buf_len, 
struct server *srv, struct
}
}

+   if (strm) {
+   struct buffer *replace = NULL;
+
+   list_for_each_entry(srv_tlv, >pp_tlvs, list) {
+   r

RE: [PATCH 1/4] MEDIUM: server: Parse generic type-value pairs as proxy-v2-options

2023-09-15 Thread Stephan, Alexander

From fb8714c5aebd7fe957264d0f2234182f55f952fe Mon Sep 17 00:00:00 2001
From: Alexander Stephan 
mailto:alexander.step...@sap.com>>
Date: Fri, 15 Sep 2023 12:38:46 +0200
Subject: [PATCH 1/4] MEDIUM: server: Parse generic type-value pairs as
 proxy-v2-options

This commit introduces a generic server-side parsing of type-value pair
arguments and allocation of a TLV list. This allows to 1) forward any TLV
type, 2) generally send out any TLV type, and 3) allows to specify concrete
values via an '=' after the type and a log fmt expression. If there is no
TLV found in the remote connection, an empty TLV is sent out.
---
 include/haproxy/server-t.h | 10 +
 src/server.c   | 76 +++---
 2 files changed, 81 insertions(+), 5 deletions(-)

diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index 1175a470e..7b93bf48f 100644
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -256,6 +256,15 @@ enum __attribute__((__packed__)) srv_ws_mode {
SRV_WS_H2,
 };

+/* Server-side TLV list, contains the types of the TLVs that should be sent 
out.
+ * Additionally, it can contain data, if specified in the config.
+ */
+struct srv_pp_tlv_list {
+   struct list list;
+   struct list fmt;
+   unsigned char type;
+};
+
 struct proxy;
 struct server {
/* mostly config or admin stuff, doesn't change often */
@@ -421,6 +430,7 @@ struct server {
struct list srv_rec_item;   /* to attach server to a srv record item */
struct list ip_rec_item;/* to attach server to a A or  record 
item */
struct ebpt_node host_dn;   /* hostdn store for srvrq and state file 
matching*/
+   struct list pp_tlvs;/* to send out PROXY protocol v2 TLVs */
struct task *srvrq_check;   /* Task testing SRV record 
expiration date for this server */
struct {
const char *file;   /* file where the section appears */
diff --git a/src/server.c b/src/server.c
index 3673340d1..08f86d030 100644
--- a/src/server.c
+++ b/src/server.c
@@ -1036,7 +1036,13 @@ static int srv_parse_proto(char **args, int *cur_arg,
 static int srv_parse_proxy_v2_options(char **args, int *cur_arg,
  struct proxy *px, struct server *newsrv, char **err)
 {
-   char *p, *n;
+   char *p = NULL, *n = NULL, *m = NULL;
+   char *key = NULL, *val = NULL;
+   char *endp = NULL;
+   unsigned char idx = 0;
+   size_t key_length = 0, val_length = 0;
+   struct srv_pp_tlv_list *srv_tlv = NULL;
+
for (p = args[*cur_arg+1]; p; p = n) {
n = strchr(p, ',');
if (n)
@@ -1061,13 +1067,61 @@ static int srv_parse_proxy_v2_options(char **args, int 
*cur_arg,
newsrv->pp_opts |= SRV_PP_V2_CRC32C;
} else if (strcmp(p, "unique-id") == 0) {
newsrv->pp_opts |= SRV_PP_V2_UNIQUE_ID;
-   } else
-   goto fail;
+   } else {
+   /* reset val in case it was set before */
+   val = NULL;
+
+   /* try to split by '=' into generic pair key value pair 
(=) */
+   m = strchr(p, '=');
+   if (m) {
+   key_length = m - p;
+   key = (char *) malloc(key_length + 1);
+   if (unlikely(!key))
+   goto fail;
+   snprintf(key, key_length + 1, "%s", p);
+
+   val_length = strlen(p) - key_length;
+   val = (char *) malloc(val_length + 1);
+   if (unlikely(!val))
+   goto fail;
+   snprintf(val, val_length + 1, "%s", m + 1);
+   }
+   else {
+   key = (char *) malloc(strlen(p) + 1);
+   if (unlikely(!key))
+   goto fail;
+   snprintf(key, strlen(p) + 1, "%s", p);
+   }
+
+   errno = 0;
+   idx = strtoul(key, , 0);
+   if (unlikely((endp && *endp) != '\0' || (key == endp) || (errno != 
0)))
+   goto fail;
+
+   /* processing is done in connection.c */
+   srv_tlv = malloc(sizeof(struct srv_pp_tlv_list));
+   if (unlikely(!srv_tlv))
+   goto fail;
+
+   srv_tlv->type = idx;
+   LIST_INIT(_tlv->fmt);
+   if (val && unlikely(!parse_logformat_string(val, px, _tlv->fmt, 
LOG_OPT_HTTP, PR_CAP_LISTEN, err)))
+   goto fail;
+
+   LIST_APPEND(>pp_tlvs, _tlv->list);
+   free(key);
+   free(val);
+   }
}
return 0;
- fail:
+fail:
+   free(key);
+   free(val);
+   free(srv_tlv);
+   errno = 0;
+
if (err)
-   memprintf(err, "'%s' : proxy v2 option not implemented", p);
+   memprintf(err, "'%s' : failed to set proxy v2 option", p);
    return ERR_ALERT | ERR_FATAL;
 }

@@ -2428,6 +2482,7 @@ struct server *new_server(struct proxy *proxy)
LIST_APPEND(_list, >global_list);
LIST_INIT(>srv_rec_item);
LIST_INIT

Re: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values

2023-09-11 Thread Willy Tarreau

Hi Alexander,

On Mon, Sep 11, 2023 at 03:44:16PM +, Stephan, Alexander wrote:
> Hi Willy and Ilya,
> 
> Sorry for the absence, I was mostly out-of-office the last week. I am really
> sorry for causing this bug.

No worries, stuff like this happens, that's why we have regtests, CI,
this development cycle, and we run all development versions in production
on haproxy.org. I could have caught it as well when merging your patches
and didn't either.

> Thanks so much Willy for fixing it as well as for the pool inspection feature
> I saw today, that looks really handy.

Yep, actually I figured it could help a lot in some contexts like CI
where we don't have the cores, or even some issue reports where we
can't reasonably ask the user to send the core nor type complex commands.
And I thought that this example was a perfect one since it triggered under
vtest, so I rolled back to the commit, applied my patches on top of it and
tried again.

> I actually debugged the code for quite some time as well, I also tried
> analyzers but sadly was not able to find it.

That's the problem with such bugs that trigger at exactly one size. You'd
have needed a fuzzer, and even then... Too much time needed to fall by
pure luck on the problem. CI and prod are much more efficient ;-)

> While looking through the CI history, I found
> https://github.com/haproxy/haproxy/runs/16184951880, also related to TLVs and
> only failing on BSD.
> I guess, this is actual flakiness then? I didn't find any related bug fix yet.

I think so because locally I'm seeing random tests fail on FreeBSD and
I don't know why. We don't monitor this one often and already found it
broken in the past. There might be a bug hitting this platform, or some
flakiness of vtest there, I don't know for now. We'll figure it before
the release anyway :-)

Cheers,
Willy

RE: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values

2023-09-11 Thread Stephan, Alexander

Hi Willy and Ilya,

Sorry for the absence, I was mostly out-of-office the last week. I am really 
sorry for causing this bug.
Thanks so much Willy for fixing it as well as for the pool inspection feature I 
saw today, that looks really handy.
I actually debugged the code for quite some time as well, I also tried 
analyzers but sadly was not able to find it.

While looking through the CI history, I found 
https://github.com/haproxy/haproxy/runs/16184951880, also related to TLVs and 
only failing on BSD.
I guess, this is actual flakiness then? I didn’t find any related bug fix yet.

Best,
Alexander

From: Илья Шипицин 
Sent: Thursday, August 31, 2023 8:56 PM
To: Willy Tarreau 
Cc: Stephan, Alexander ; haproxy@formilux.org
Subject: Re: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY 
protocol v2 TLV values

You don't often get email from 
chipits...@gmail.com<mailto:chipits...@gmail.com>. Learn why this is 
important<https://aka.ms/LearnAboutSenderIdentification>
cirrus-ci backtrace

freebsd (cirrus-ci) crash · Issue #2275 · haproxy/haproxy 
(github.com)<https://github.com/haproxy/haproxy/issues/2275>

as usual, I'll send CI improvements once polished

чт, 31 авг. 2023 г. в 18:22, Илья Шипицин 
mailto:chipits...@gmail.com>>:
while trying to enable "gdb bt" on cirrus-ci, I noticed that we have similar 
crashes on musl (where gdb implemented already)

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

ср, 30 авг. 2023 г. в 05:29, Willy Tarreau mailto:w...@1wt.eu>>:
On Tue, Aug 29, 2023 at 11:16:32PM +0200,  ??? wrote:
> ??, 29 ???. 2023 ?. ? 16:45, Willy Tarreau mailto:w...@1wt.eu>>:
>
> > On Tue, Aug 29, 2023 at 04:31:31PM +0200, Willy Tarreau wrote:
> > > On Tue, Aug 29, 2023 at 02:16:55PM +, Stephan, Alexander wrote:
> > > > However, I noticed there is a problem now with the FreeBSD test. Have
> > you
> > > > already looked into it?
> > >
> > > Ah no, I had not noticed. I first pushed into a temporary branch and
> > > everything was OK so I pushed into master again without checking since
> > > it was the exact same commit.
> > >
> > > > I was not able to reproduce it for now. Looks like the test passes but
> > it
> > > > crashes afterwards? Maybe some SEGFAULT. Not sure... the CI on your
> > branch
> > > > looked good iirc.
> > >
> > > Hmmm no, signal 4 is SIGILL, we're using it for our asserts (BUG_ON()).
> > > I never know how to retrieve the stderr log from these tests, there are
> > > too many layers for me, I've been explained several times but cannot
> > > memorize it.
> > >
> > > I'm rebuilding here on a freebsd machine, hoping to see it happen again.
> > >
> > > Note that we cannot rule out a transient issue such as a temporary memory
> > > shortage or too long a CPU pause as such VMs are usually overloaded.
> > > Maybe it has nothing to do with your new test but just randomly triggered
> > > there, or maybe it put the light on a corner case. At least it's not a
> > > segfault or such a crash that would indicate a pointer misuse.
> >
> > I'm getting totally random results from vtest on FreeBSD, I don't know
> > why. I even tried to limit to one parallel process and am still getting
> > on average 2 errors per run, most always on SSL but not only. Some of
> > them include connection failures (sC) with 503 being returned and showing
> > "Timeout during SSL handshake". I'm not getting the SIGILL though. Thus
> > I don't know what to think about it, especially since it previously worked.
> > We'll see if it happens again on next push.
> >
> > Willy
> >
>
> we can disable cirrus-ci, not sure what is the purpose of randomly failing
> CI :)

I think it's still used for arm builds or something like this. Regardless,
given that freebsd randomly fails on my local platform,there might be
something else. I remember that cirrus used to fail some time ago, I
don't know what its status is right now.

Willy

Re: Haproxy 2.8 with Proxy Protocol v2 does not close connections

2023-09-07 Thread Lukas Tribus

On Thu, 7 Sept 2023 at 14:03, Tom Braarup  wrote:
>
> Hello,
>
> After upgrading Haproxy from 2.7 to 2.8, with Nginx (1.25.0) as
> backends and Proxy Protocol v2, the connections are not closed,
> CLOSE_WAIT is increasing over time. No configuration changes apart from
> the Haproxy version.

2.8.3 was just released with several important fixes. I suggest you
give it a try when it hits Vincent's PPA.


Lukas

Haproxy 2.8 with Proxy Protocol v2 does not close connections

2023-09-07 Thread Tom Braarup

Hello,

After upgrading Haproxy from 2.7 to 2.8, with Nginx (1.25.0) as
backends and Proxy Protocol v2, the connections are not closed,
CLOSE_WAIT is increasing over time. No configuration changes apart from
the Haproxy version. 

Using Haproxy as backend will not create the same issue.

Not Working: Haproxy 2.8 -> Nginx (TLS Offloading)   -> Varnish 
Working: Haproxy 2.8 -> Haproxy (TLS Offloading) -> Varnish
 


Ubuntu 22.04
HAProxy version 2.7.10-1ppa1~jammy 2023/08/12
or 
HAProxy version 2.8.2-1ppa1~jammy 2023/08/12

(Thanks again Vincent Bernat for the PPA)


Config:

global
  crt-base  /etc/ssl/private
  group  haproxy
  log  127.0.0.1 local0
  ssl-default-bind-ciphers  ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-
AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-
SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-
RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384
  ssl-default-bind-ciphersuites 
TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA
256
  ssl-default-bind-options  prefer-client-ciphers ssl-min-ver TLSv1.2
no-tls-tickets
  ssl-default-server-ciphers  ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-
AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-
SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-
RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384
  ssl-default-server-ciphersuites 
TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA
256
  ssl-default-server-options  ssl-min-ver TLSv1.2 no-tls-tickets
  stats  socket /var/run/haproxy.sock  user haproxyapi  group
haproxyapi  mode 660  level admin  expose-fd listeners
  tune.ssl.default-dh-param  2048 
  user  haproxy

defaults
  log  global
  maxconn  8000
  option  redispatch
  retries  3
  stats  enable
  timeout  http-request 10s
  timeout  queue 1m
  timeout  connect 10s
  timeout  client 1m
  timeout  server 1m
  timeout  check 10s

listen http
  bind [redacted]:80 interface ens224
  mode tcp
  balance roundrobin
  log global
  option tcplog
  server web1 [redacted]:2080 check send-proxy-v2
  server web2 [redacted]:2080 check send-proxy-v2
  server web3 [redacted]:2080 check send-proxy-v2
  server web4 [redacted]:2080 check send-proxy-v2


listen https
  bind [redacted]:443 interface ens224
  mode tcp
  balance roundrobin
  log global
  option tcplog
  server web1 [redacted]:2443 check send-proxy-v2
  server web2 [redacted]:2443 check send-proxy-v2
  server web3 [redacted]:2443 check send-proxy-v2
  server web4 [redacted]:2443 check send-proxy-v2


-- 
Tom Braarup
Senior IT-Administrator / Commercial and Tech Operations

Re: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values

2023-08-31 Thread Илья Шипицин

cirrus-ci backtrace

freebsd (cirrus-ci) crash · Issue #2275 · haproxy/haproxy (github.com)


as usual, I'll send CI improvements once polished

чт, 31 авг. 2023 г. в 18:22, Илья Шипицин :

> while trying to enable "gdb bt" on cirrus-ci, I noticed that we have
> similar crashes on musl (where gdb implemented already)
>
> https://github.com/haproxy/haproxy/issues/2274
>
> ср, 30 авг. 2023 г. в 05:29, Willy Tarreau :
>
>> On Tue, Aug 29, 2023 at 11:16:32PM +0200,  ??? wrote:
>> > ??, 29 ???. 2023 ?. ? 16:45, Willy Tarreau :
>> >
>> > > On Tue, Aug 29, 2023 at 04:31:31PM +0200, Willy Tarreau wrote:
>> > > > On Tue, Aug 29, 2023 at 02:16:55PM +, Stephan, Alexander wrote:
>> > > > > However, I noticed there is a problem now with the FreeBSD test.
>> Have
>> > > you
>> > > > > already looked into it?
>> > > >
>> > > > Ah no, I had not noticed. I first pushed into a temporary branch and
>> > > > everything was OK so I pushed into master again without checking
>> since
>> > > > it was the exact same commit.
>> > > >
>> > > > > I was not able to reproduce it for now. Looks like the test
>> passes but
>> > > it
>> > > > > crashes afterwards? Maybe some SEGFAULT. Not sure... the CI on
>> your
>> > > branch
>> > > > > looked good iirc.
>> > > >
>> > > > Hmmm no, signal 4 is SIGILL, we're using it for our asserts
>> (BUG_ON()).
>> > > > I never know how to retrieve the stderr log from these tests, there
>> are
>> > > > too many layers for me, I've been explained several times but cannot
>> > > > memorize it.
>> > > >
>> > > > I'm rebuilding here on a freebsd machine, hoping to see it happen
>> again.
>> > > >
>> > > > Note that we cannot rule out a transient issue such as a temporary
>> memory
>> > > > shortage or too long a CPU pause as such VMs are usually overloaded.
>> > > > Maybe it has nothing to do with your new test but just randomly
>> triggered
>> > > > there, or maybe it put the light on a corner case. At least it's
>> not a
>> > > > segfault or such a crash that would indicate a pointer misuse.
>> > >
>> > > I'm getting totally random results from vtest on FreeBSD, I don't know
>> > > why. I even tried to limit to one parallel process and am still
>> getting
>> > > on average 2 errors per run, most always on SSL but not only. Some of
>> > > them include connection failures (sC) with 503 being returned and
>> showing
>> > > "Timeout during SSL handshake". I'm not getting the SIGILL though.
>> Thus
>> > > I don't know what to think about it, especially since it previously
>> worked.
>> > > We'll see if it happens again on next push.
>> > >
>> > > Willy
>> > >
>> >
>> > we can disable cirrus-ci, not sure what is the purpose of randomly
>> failing
>> > CI :)
>>
>> I think it's still used for arm builds or something like this. Regardless,
>> given that freebsd randomly fails on my local platform,there might be
>> something else. I remember that cirrus used to fail some time ago, I
>> don't know what its status is right now.
>>
>> Willy
>>
>

Re: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values

2023-08-31 Thread Илья Шипицин

while trying to enable "gdb bt" on cirrus-ci, I noticed that we have
similar crashes on musl (where gdb implemented already)

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

ср, 30 авг. 2023 г. в 05:29, Willy Tarreau :

> On Tue, Aug 29, 2023 at 11:16:32PM +0200,  ??? wrote:
> > ??, 29 ???. 2023 ?. ? 16:45, Willy Tarreau :
> >
> > > On Tue, Aug 29, 2023 at 04:31:31PM +0200, Willy Tarreau wrote:
> > > > On Tue, Aug 29, 2023 at 02:16:55PM +, Stephan, Alexander wrote:
> > > > > However, I noticed there is a problem now with the FreeBSD test.
> Have
> > > you
> > > > > already looked into it?
> > > >
> > > > Ah no, I had not noticed. I first pushed into a temporary branch and
> > > > everything was OK so I pushed into master again without checking
> since
> > > > it was the exact same commit.
> > > >
> > > > > I was not able to reproduce it for now. Looks like the test passes
> but
> > > it
> > > > > crashes afterwards? Maybe some SEGFAULT. Not sure... the CI on your
> > > branch
> > > > > looked good iirc.
> > > >
> > > > Hmmm no, signal 4 is SIGILL, we're using it for our asserts
> (BUG_ON()).
> > > > I never know how to retrieve the stderr log from these tests, there
> are
> > > > too many layers for me, I've been explained several times but cannot
> > > > memorize it.
> > > >
> > > > I'm rebuilding here on a freebsd machine, hoping to see it happen
> again.
> > > >
> > > > Note that we cannot rule out a transient issue such as a temporary
> memory
> > > > shortage or too long a CPU pause as such VMs are usually overloaded.
> > > > Maybe it has nothing to do with your new test but just randomly
> triggered
> > > > there, or maybe it put the light on a corner case. At least it's not
> a
> > > > segfault or such a crash that would indicate a pointer misuse.
> > >
> > > I'm getting totally random results from vtest on FreeBSD, I don't know
> > > why. I even tried to limit to one parallel process and am still getting
> > > on average 2 errors per run, most always on SSL but not only. Some of
> > > them include connection failures (sC) with 503 being returned and
> showing
> > > "Timeout during SSL handshake". I'm not getting the SIGILL though. Thus
> > > I don't know what to think about it, especially since it previously
> worked.
> > > We'll see if it happens again on next push.
> > >
> > > Willy
> > >
> >
> > we can disable cirrus-ci, not sure what is the purpose of randomly
> failing
> > CI :)
>
> I think it's still used for arm builds or something like this. Regardless,
> given that freebsd randomly fails on my local platform,there might be
> something else. I remember that cirrus used to fail some time ago, I
> don't know what its status is right now.
>
> Willy
>

Re: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values

On Tue, Aug 29, 2023 at 11:16:32PM +0200,  ??? wrote:
> ??, 29 ???. 2023 ?. ? 16:45, Willy Tarreau :
> 
> > On Tue, Aug 29, 2023 at 04:31:31PM +0200, Willy Tarreau wrote:
> > > On Tue, Aug 29, 2023 at 02:16:55PM +, Stephan, Alexander wrote:
> > > > However, I noticed there is a problem now with the FreeBSD test. Have
> > you
> > > > already looked into it?
> > >
> > > Ah no, I had not noticed. I first pushed into a temporary branch and
> > > everything was OK so I pushed into master again without checking since
> > > it was the exact same commit.
> > >
> > > > I was not able to reproduce it for now. Looks like the test passes but
> > it
> > > > crashes afterwards? Maybe some SEGFAULT. Not sure... the CI on your
> > branch
> > > > looked good iirc.
> > >
> > > Hmmm no, signal 4 is SIGILL, we're using it for our asserts (BUG_ON()).
> > > I never know how to retrieve the stderr log from these tests, there are
> > > too many layers for me, I've been explained several times but cannot
> > > memorize it.
> > >
> > > I'm rebuilding here on a freebsd machine, hoping to see it happen again.
> > >
> > > Note that we cannot rule out a transient issue such as a temporary memory
> > > shortage or too long a CPU pause as such VMs are usually overloaded.
> > > Maybe it has nothing to do with your new test but just randomly triggered
> > > there, or maybe it put the light on a corner case. At least it's not a
> > > segfault or such a crash that would indicate a pointer misuse.
> >
> > I'm getting totally random results from vtest on FreeBSD, I don't know
> > why. I even tried to limit to one parallel process and am still getting
> > on average 2 errors per run, most always on SSL but not only. Some of
> > them include connection failures (sC) with 503 being returned and showing
> > "Timeout during SSL handshake". I'm not getting the SIGILL though. Thus
> > I don't know what to think about it, especially since it previously worked.
> > We'll see if it happens again on next push.
> >
> > Willy
> >
> 
> we can disable cirrus-ci, not sure what is the purpose of randomly failing
> CI :)

I think it's still used for arm builds or something like this. Regardless,
given that freebsd randomly fails on my local platform,there might be
something else. I remember that cirrus used to fail some time ago, I
don't know what its status is right now.

Willy

Re: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values

2023-08-29 Thread Илья Шипицин

вт, 29 авг. 2023 г. в 16:45, Willy Tarreau :

> On Tue, Aug 29, 2023 at 04:31:31PM +0200, Willy Tarreau wrote:
> > On Tue, Aug 29, 2023 at 02:16:55PM +, Stephan, Alexander wrote:
> > > However, I noticed there is a problem now with the FreeBSD test. Have
> you
> > > already looked into it?
> >
> > Ah no, I had not noticed. I first pushed into a temporary branch and
> > everything was OK so I pushed into master again without checking since
> > it was the exact same commit.
> >
> > > I was not able to reproduce it for now. Looks like the test passes but
> it
> > > crashes afterwards? Maybe some SEGFAULT. Not sure... the CI on your
> branch
> > > looked good iirc.
> >
> > Hmmm no, signal 4 is SIGILL, we're using it for our asserts (BUG_ON()).
> > I never know how to retrieve the stderr log from these tests, there are
> > too many layers for me, I've been explained several times but cannot
> > memorize it.
> >
> > I'm rebuilding here on a freebsd machine, hoping to see it happen again.
> >
> > Note that we cannot rule out a transient issue such as a temporary memory
> > shortage or too long a CPU pause as such VMs are usually overloaded.
> > Maybe it has nothing to do with your new test but just randomly triggered
> > there, or maybe it put the light on a corner case. At least it's not a
> > segfault or such a crash that would indicate a pointer misuse.
>
> I'm getting totally random results from vtest on FreeBSD, I don't know
> why. I even tried to limit to one parallel process and am still getting
> on average 2 errors per run, most always on SSL but not only. Some of
> them include connection failures (sC) with 503 being returned and showing
> "Timeout during SSL handshake". I'm not getting the SIGILL though. Thus
> I don't know what to think about it, especially since it previously worked.
> We'll see if it happens again on next push.
>
> Willy
>

we can disable cirrus-ci, not sure what is the purpose of randomly failing
CI :)

Re: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values

On Tue, Aug 29, 2023 at 04:31:31PM +0200, Willy Tarreau wrote:
> On Tue, Aug 29, 2023 at 02:16:55PM +, Stephan, Alexander wrote:
> > However, I noticed there is a problem now with the FreeBSD test. Have you
> > already looked into it?
> 
> Ah no, I had not noticed. I first pushed into a temporary branch and
> everything was OK so I pushed into master again without checking since
> it was the exact same commit.
> 
> > I was not able to reproduce it for now. Looks like the test passes but it
> > crashes afterwards? Maybe some SEGFAULT. Not sure... the CI on your branch
> > looked good iirc. 
> 
> Hmmm no, signal 4 is SIGILL, we're using it for our asserts (BUG_ON()).
> I never know how to retrieve the stderr log from these tests, there are
> too many layers for me, I've been explained several times but cannot
> memorize it.
> 
> I'm rebuilding here on a freebsd machine, hoping to see it happen again.
> 
> Note that we cannot rule out a transient issue such as a temporary memory
> shortage or too long a CPU pause as such VMs are usually overloaded.
> Maybe it has nothing to do with your new test but just randomly triggered
> there, or maybe it put the light on a corner case. At least it's not a
> segfault or such a crash that would indicate a pointer misuse.

I'm getting totally random results from vtest on FreeBSD, I don't know
why. I even tried to limit to one parallel process and am still getting
on average 2 errors per run, most always on SSL but not only. Some of
them include connection failures (sC) with 503 being returned and showing
"Timeout during SSL handshake". I'm not getting the SIGILL though. Thus
I don't know what to think about it, especially since it previously worked.
We'll see if it happens again on next push.

Willy

Re: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values

On Tue, Aug 29, 2023 at 02:16:55PM +, Stephan, Alexander wrote:
> However, I noticed there is a problem now with the FreeBSD test. Have you
> already looked into it?

Ah no, I had not noticed. I first pushed into a temporary branch and
everything was OK so I pushed into master again without checking since
it was the exact same commit.

> I was not able to reproduce it for now. Looks like the test passes but it
> crashes afterwards? Maybe some SEGFAULT. Not sure... the CI on your branch
> looked good iirc. 

Hmmm no, signal 4 is SIGILL, we're using it for our asserts (BUG_ON()).
I never know how to retrieve the stderr log from these tests, there are
too many layers for me, I've been explained several times but cannot
memorize it.

I'm rebuilding here on a freebsd machine, hoping to see it happen again.

Note that we cannot rule out a transient issue such as a temporary memory
shortage or too long a CPU pause as such VMs are usually overloaded.
Maybe it has nothing to do with your new test but just randomly triggered
there, or maybe it put the light on a corner case. At least it's not a
segfault or such a crash that would indicate a pointer misuse.

Willy

RE: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values

2023-08-29 Thread Stephan, Alexander

Hi Willy,

> And I was wrong, they were indeed for the first one. However I had to also 
> remove the NOT_LAST from the intermediate patches using the list_for_each().
> I put quotes around the symbolic names in the doc to make it  clearer which 
> one was to be used and which one it corresponds to, as I had a moment of 
> hesitation when reading it the first time, so I profitted from this first 
> encounter to try to further limit doubts.
Thanks for the adjustments and clarifications. 

> That's now merged, thanks again!
Very happy that is merged now.

However, I noticed there is a problem now with the FreeBSD test. Have you 
already looked into it?
I was not able to reproduce it for now. Looks like the test passes but it 
crashes afterwards? Maybe some SEGFAULT. Not sure... the CI on your branch 
looked good iirc. 

Best,
Alexander

Re: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values

On Tue, Aug 29, 2023 at 03:15:48PM +0200, Willy Tarreau wrote:
> Overall yes. There are just two small parts in the first patch that are
> for the immediately following patches ("refactor...") that I'm going to
> move there.

And I was wrong, they were indeed for the first one. However I had to
also remove the NOT_LAST from the intermediate patches using the
list_for_each(). I put quotes around the symbolic names in the doc to
make it clearer which one was to be used and which one it corresponds
to, as I had a moment of hesitation when reading it the first time, so
I profitted from this first encounter to try to further limit doubts.

> I'm doing that and merging it, or I'll get back to you if I have more
> questions. Thank you!

That's now merged, thanks again!
Willy

Re: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values

Hi Alexander,

On Mon, Aug 28, 2023 at 12:38:45PM +, Stephan, Alexander wrote:
> > I'm fine with this, however I find that the doc is not very clear about
> > what is permitted
> I agree that doc needs some more details. I added the note about the
> iterations and described all the symbolic constants, see at the end of this
> mail.

Thanks.

> > Unless you're having any objection, I'm going to flip type and len below:
> Sorry, that was an oversight on my end. Sure, go ahead. I actually thought I
> would have arranged it that way, but it somehow slipped my view.

No worries, that's what reviews are made for, isn't it ? ;-)

> > In this case it could be clarified in the doc that the sample fetch
> > functions for authority/uniqueid only return the first of each, and that
> > fc_pp_tlv() iterates over all occurrences of the requested ID. This would
> > then place a clear separation between trying to extract THE authority, or
> > checking ALL TLVs of type AUTHORITY.
> 
> > What do you think ?
> Good idea, agreed. That also aligns with my goals to keep the existing
> behavior as much as possible.

OK great.

> > If you're OK with this, I'd appreciate it if you could send a few informal
> > incremental patches that I'd squash into yours.
> I'm OK with this. :-) The first patch (0001) should be squashed into the
> commit that introduces the fetch fc_pp_tlv.
> I guess it fits there because it actually retains and documents prior
> behavior. The behavior regarding duplicates is also already present,
> therefore I already added corresponding docs there.
> The second one  (0002) could then be applied to the last patch with the
> introduction of the constants. Would that work for you?

Overall yes. There are just two small parts in the first patch that are
for the immediately following patches ("refactor...") that I'm going to
move there.

I'm doing that and merging it, or I'll get back to you if I have more
questions. Thank you!

Willy

RE: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values

2023-08-28 Thread Stephan, Alexander

Hi Willy,

> finally back to this! Overall it's a great and very clean series, I really 
> want to thank you for this high quality work!
Thanks for the compliment, really glad to hear! :)

> Yeah it initially gave me a bit of head scratching when reading this part but 
> I understood what you did and find that it pretty much makes sense after all.
> It's not necessarily a hack given that what you've done consists in passing a 
> pre-parsed argument internally. It's not much different from the few cases 
> where we emulate older functions or actions using new mechanisms, building 
> some config elements on the fly just to parse them.
Okay, great if there is already similar code. Hack was a bit of an 
overstatement by me, at least if works a bit different than usually.

> I'm fine with this, however I find that the doc is not very clear about what 
> is permitted
I agree that doc needs some more details. I added the note about the iterations 
and described all the symbolic constants, see at the end of this mail.

> Unless you're having any objection, I'm going to flip type and len below:
Sorry, that was an oversight on my end. Sure, go ahead. I actually thought I 
would have arranged it that way, but it somehow slipped my view.

> I tend to *prefer* to have them separately for long series that take a lot of 
> time to review and cannot be done at once. But this series could be reviewed 
> at once, most patches remain small enough so that's no big deal.
OK, great. But I try to keep it in mind for future work anyway.

> In this case it could be clarified in the doc that the sample fetch functions 
> for authority/uniqueid only return the first of each, and that
> fc_pp_tlv() iterates over all occurrences of the requested ID. This would 
> then place a clear separation between trying to extract THE authority, or 
> checking ALL TLVs of type AUTHORITY.

> What do you think ?
Good idea, agreed. That also aligns with my goals to keep the existing behavior 
as much as possible.

> If you're OK with this, I'd appreciate it if you could send a few informal 
> incremental patches that I'd squash into yours.
I'm OK with this. :-) The first patch (0001) should be squashed into the commit 
that introduces the fetch fc_pp_tlv.
I guess it fits there because it actually retains and documents prior behavior. 
The behavior regarding duplicates is also already present,
therefore I already added corresponding docs there.
The second one  (0002) could then be applied to the last patch with the 
introduction of the constants. Would that work for you?

Best,
Alexander


0002-Extend-docs-of-fc_pp_tlv-with-constants.patch
Description: 0002-Extend-docs-of-fc_pp_tlv-with-constants.patch


0001-Restrict-fc_pp_authority-and-fc_pp_unique_id-to-firs.patch
Description:  0001-Restrict-fc_pp_authority-and-fc_pp_unique_id-to-firs.patch

Re: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values

2023-08-28 Thread Willy Tarreau

Hi Alexander,

finally back to this! Overall it's a great and very clean series, I really
want to thank you for this high quality work!

On Wed, Aug 16, 2023 at 04:24:21PM +, Stephan, Alexander wrote:
> I was not able to use a check function for authority and unique_id without
> modifying sample.c or allowing an argument. As far as I can tell, the check
> function is only executed for sample fetches that have not specified 0 for
> the arg function. I think it's okay this way since, semantically, those
> function do not handle arguments, so doing the argument setup internally
> makes sense IMO.

Yeah it initially gave me a bit of head scratching when reading this part
but I understood what you did and find that it pretty much makes sense
after all.

> If there is way to adjust this without any hacks, I will of course apply it.

It's not necessarily a hack given that what you've done consists in passing
a pre-parsed argument internally. It's not much different from the few cases
where we emulate older functions or actions using new mechanisms, building
some config elements on the fly just to parse them.

> For now, I have appended them to this message to not cause to much spam on
> the list while also keeping the previous, related discussion.
> If I should send them individually, please tell me and I will do so.

I tend to *prefer* to have them separately for long series that take a lot
of time to review and cannot be done at once. But this series could be
reviewed at once, most patches remain small enough so that's no big deal.

> Restructuring, especially in regard to pooling lead to quite a bit of
> changes, but the overall logic still applies and should hopefully be
> relatively straight forward.
> Actually, the code even got simpler in many places! :)

I noticed as well. I think I understood it all, which is a good point.

> BTW, I also added some TLV type constants in the 6th patch, feel free to
> merge it or ignore it. I think it helps with readability and is not really
> risky.

I'm fine with this, however I find that the doc is not very clear about
what is permitted:

 fc_pp_tlv() : string
   Returns the TLV value for the given TLV ID or type constant sent by the
   client in the PROXY protocol header, if any. TLV constants correspond
   to their type suffix as specified in the PPv2 spec, e.g., AUTHORITY.

It's not clear to me as a user that this  argument supports a
numeric value, nor what is the allowed range (I don't even know
personally). And given that you added a number of special keywords,
it's important that those recognized by the code are also mentioned
here. I would suggest something around:

 ... the given TLV ID which must either be a numeric value between
 XXX and YYY included, or one of the supported following symbolic
 names that correspond to the TLV constant suffixes in the PPv2 spec:
 AUTHORITY:x, ...

I would appreciate it if you can just reword this part with the ranges,
names and values according to what you know about it, I'll happily
apply it directly into your last patch.

> If there should be a nit that you quickly want to change, feel free to. I am
> not upset about it at all.

Unless you're having any objection, I'm going to flip type and len below:

 struct conn_tlv_list {
struct list list;
unsigned char type;
unsigned short len; // 65535 should be more than enough!
char value[0];
 } __attribute__((packed));

These result in  being unaligned, which is less clean on some archs,
and would also make the compiler complain if a pointer to len is ever
passed to a function taking a short*.

Finally, I noticed a behaviour change and I don't know if it's intentional
or not, but I think it needs to be discussed and possibly also documented.
Prior to your patchset, the authority and uniqueid were unique (I don't
know which of the first or last was kept, and I haven't checked but it
could even be possible that in case of duplicate it results in a memory
leak). But at least a single one was used. Now since you're setting
NOT_LAST on the sample, multiple such samples can be checked, e.g. in an
ACL that comes back and retrieves the next TLV of the same type when looking
for something. While I find this good, I do not think it is a good idea to
support that for important stuff like AUTHORITY or UNIQUEID: if an upstream
equipment can be fooled into sending more than one of each, the behavior
can be particularly confusing and may even be abused. I think this could
be addressed like this:

  /* fetch the authority TLV from a PROXY protocol header */
  int smp_fetch_fc_pp_authority(const struct arg *args, struct sample *smp, 
const char *kw, void *private)
  {
struct arg tlv_arg;
int ret;

set_tlv_arg(PP2_TYPE_AUTHORITY, _arg);
ret = smp_fetch_fc_pp_tlv(_arg, smp, kw, private);
smp->flags &= ~SMP_F_NOT_LAST; // return only th

Re: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values

2023-08-25 Thread Willy Tarreau

Hi Alexander,

On Fri, Aug 25, 2023 at 09:34:08AM +, Stephan, Alexander wrote:
> Hi Willy,
> 
> Did you already have the chance to take a look at the updated patches?
> No hurry though, I just wanted to make sure that the message didn't get lost.

Not yet, I'm still burried under annoying bugs for now. I was just telling
a coworker that I intend to review your work early next week. I'm fairly
confident but it requires time.

Thanks!
Willy

RE: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values

2023-08-25 Thread Stephan, Alexander

Hi Willy,

Did you already have the chance to take a look at the updated patches?
No hurry though, I just wanted to make sure that the message didn't get lost.

As mentioned, I am aware that sending individual patches is better in the 
common case.
If that is a problem here, please just let me know.

Best,
Alexander

RE: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values

2023-08-16 Thread Stephan, Alexander

Hi Willy,

Thanks for the clarifications. I've implemented your requested changes and 
split my changes in 6 consecutive patches.

I was not able to use a check function for authority and unique_id without 
modifying sample.c or allowing an argument.
As far as I can tell, the check function is only executed for sample fetches 
that have not specified 0 for the arg function.
I think it's okay this way since, semantically, those function do not handle 
arguments, so doing the argument setup internally makes sense IMO.
If there is way to adjust this without any hacks, I will of course apply it.

For now, I have appended them to this message to not cause to much spam on the 
list while also keeping the previous, related discussion.
If I should send them individually, please tell me and I will do so.

Restructuring, especially in regard to pooling lead to quite a bit of changes, 
but the overall logic still applies and should hopefully be relatively straight 
forward.
Actually, the code even got simpler in many places! :)

BTW, I also added some TLV type constants in the 6th patch, feel free to merge 
it or ignore it.
I think it helps with readability and is not really risky.

If there should be a nit that you quickly want to change, feel free to. I am 
not upset about it at all.

Best,
Alexander

-Original Message-
From: Willy Tarreau  
Sent: Sunday, August 13, 2023 10:01 AM
To: Stephan, Alexander 
Cc: haproxy@formilux.org
Subject: Re: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY 
protocol v2 TLV values

[You don't often get email from w...@1wt.eu. Learn why this is important at 
https://aka.ms/LearnAboutSenderIdentification ]

Hi Alexander,

On Fri, Aug 11, 2023 at 02:08:37PM +, Stephan, Alexander wrote:
> Hi Willy,
>
> Thanks for the nice, detailed feedback.
> Overall, I agree with all of your listed points, so no need for further 
> discussions. ?
> I will hopefully send the separated patches at the beginning of next week.

OK! No rush anyway, such changes having a low impact can be merged late in the 
cycle if needed, so take your time.

> Just a comment and two small questions to make sure I got things correct:
>
> > As such I'd like them to be renamed to remove this confusion.
> > Maybe just call them HA_PP2_* ?
>
> Yes, such a prefix will clean it up quite nicely IMO. Will add that to 
> the first patch of the series.

Thanks.

> > [...]
> > It may even encourage us to create
> > smaller pools if ever deemed really useful (e.g. a 4- or 8- byte 
> > load balancing hash key for end-to-end consistency would only take a 
> > total of 32 or 40, malloc included).
>
> Just to make sure: Right now, we don't want to optimize further for 
> small TLVs other than removing the second pool for the values and 
> using the new struct with the VLA to reduce the overhead.
> For normal use, a pool with 160 B could  be used now to accommodate 
> for the new conn_tlv_list struct and 128 B TLVs (e.g., UNIQUE_ID)?
> For the authority type, it would then be a 255 + 32 B sized pool? 
> Maybe that could be used for the value range 128 <= x <= 255, and 
> then, for > 255, malloc?

Yes I think that will do the job (more like 128 < x < 256 BTW). I think you'd 
rather just focus on the type size (like you did) and not on the type itself, 
so that pools will automatically fit (i.e. have
128+sizeof(conn_tlv_list) and 256+sizeof() and the rest is for malloc().

> Other, smaller pools are future work?

Yes, as I'm not sure they're really that much used, and it will be easy to 
create smaller pools if we figure they're needed.

> >struct conn_tlv_list {
> >   struct list list;
> >unsigned short len; // 65535 should be more than enough!
> >unsigned char type;
> >char contents[0];
>  >   };
>
> I am also a bit confused about the second struct variant of this. IMO 
> this is the optimal struct layout in regards to size, that I would like to 
> use.
> What is the other struct for, where `len` and `type` are switched?

Ah, at first I didn't know what you were talking about, I remember having added 
the type while writing the message, it's just that I poorly pasted it the 
second time :-)  It's better to keep it as above, where types are better 
aligned and leave no hole. Hmmm BTW the struct will be padded, so you should 
use offsetof(conn_tlv_list, contents) rather than
sizeof(conn_tlv_list) for the size calculations. Or you can mark the struct 
with __attribute__((packed)), it will do the job as well. It's up to you.

> Thanks again for your efforts, it's really interesting for me to work 
> on HAProxy.

You're welcome, do not hesitate to send any question you may have.

Cheers,
Willy

0001-CLEANUP-MINOR-connection-Improve-consistency-of-PPv2.patch
Description:  0001-CLEANUP-MINOR-connection-Improve

Re: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values

2023-08-13 Thread Willy Tarreau

Hi Alexander,

On Fri, Aug 11, 2023 at 02:08:37PM +, Stephan, Alexander wrote:
> Hi Willy,
> 
> Thanks for the nice, detailed feedback.
> Overall, I agree with all of your listed points, so no need for further 
> discussions. ?
> I will hopefully send the separated patches at the beginning of next week.

OK! No rush anyway, such changes having a low impact can be merged late
in the cycle if needed, so take your time.

> Just a comment and two small questions to make sure I got things correct:
> 
> > As such I'd like them to be renamed to remove this confusion.
> > Maybe just call them HA_PP2_* ?
> 
> Yes, such a prefix will clean it up quite nicely IMO. Will add that to the
> first patch of the series.

Thanks.

> > [...]
> > It may even encourage us to create
> > smaller pools if ever deemed really useful (e.g. a 4- or 8- byte
> > load balancing hash key for end-to-end consistency would only
> > take a total of 32 or 40, malloc included).
> 
> Just to make sure: Right now, we don't want to optimize further for small
> TLVs other than removing the second pool for the values and using the new
> struct with the VLA to reduce the overhead.
> For normal use, a pool with 160 B could  be used now to accommodate for the
> new conn_tlv_list struct and 128 B TLVs (e.g., UNIQUE_ID)?
> For the authority type, it would then be a 255 + 32 B sized pool? Maybe that
> could be used for the value range 128 <= x <= 255, and then, for > 255,
> malloc?

Yes I think that will do the job (more like 128 < x < 256 BTW). I think
you'd rather just focus on the type size (like you did) and not on the
type itself, so that pools will automatically fit (i.e. have
128+sizeof(conn_tlv_list) and 256+sizeof() and the rest is for malloc().

> Other, smaller pools are future work?

Yes, as I'm not sure they're really that much used, and it will be easy
to create smaller pools if we figure they're needed.

> >struct conn_tlv_list {
> >   struct list list;
> >unsigned short len; // 65535 should be more than enough!
> >unsigned char type;
> >char contents[0];
>  >   };
> 
> I am also a bit confused about the second struct variant of this. IMO this is
> the optimal struct layout in regards to size, that I would like to use.
> What is the other struct for, where `len` and `type` are switched?

Ah, at first I didn't know what you were talking about, I remember having
added the type while writing the message, it's just that I poorly pasted
it the second time :-)  It's better to keep it as above, where types are
better aligned and leave no hole. Hmmm BTW the struct will be padded, so
you should use offsetof(conn_tlv_list, contents) rather than
sizeof(conn_tlv_list) for the size calculations. Or you can mark the struct
with __attribute__((packed)), it will do the job as well. It's up to you.

> Thanks again for your efforts, it's really interesting for me to work on
> HAProxy.

You're welcome, do not hesitate to send any question you may have.

Cheers,
Willy

RE: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values

2023-08-11 Thread Stephan, Alexander

Hi Willy,

Thanks for the nice, detailed feedback.
Overall, I agree with all of your listed points, so no need for further 
discussions. 
I will hopefully send the separated patches at the beginning of next week.

Just a comment and two small questions to make sure I got things correct:

> As such I'd like them to be renamed to remove this confusion.
> Maybe just call them HA_PP2_* ?

Yes, such a prefix will clean it up quite nicely IMO. Will add that to the 
first patch of the series.

> [...]
> It may even encourage us to create
> smaller pools if ever deemed really useful (e.g. a 4- or 8- byte
> load balancing hash key for end-to-end consistency would only
> take a total of 32 or 40, malloc included).

Just to make sure: Right now, we don't want to optimize further for small TLVs 
other than removing the second pool for the values and using the new struct 
with the VLA to reduce the overhead.
For normal use, a pool with 160 B could  be used now to accommodate for the new 
conn_tlv_list struct and 128 B TLVs (e.g., UNIQUE_ID)?
For the authority type, it would then be a 255 + 32 B sized pool? Maybe that 
could be used for the value range 128 <= x <= 255, and then, for > 255, malloc?
Other, smaller pools are future work?

>struct conn_tlv_list {
>   struct list list;
>unsigned short len; // 65535 should be more than enough!
>unsigned char type;
>char contents[0];
 >   };

I am also a bit confused about the second struct variant of this. IMO this is 
the optimal struct layout in regards to size, that I would like to use.
What is the other struct for, where `len` and `type` are switched?

Thanks again for your efforts, it's really interesting for me to work on 
HAProxy.

Best,
Alexander
-Original Message-
From: Willy Tarreau  
Sent: Thursday, August 10, 2023 9:18 AM
To: Stephan, Alexander 
Cc: haproxy@formilux.org
Subject: Re: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY 
protocol v2 TLV values

[You don't often get email from w...@1wt.eu. Learn why this is important at 
https://aka.ms/LearnAboutSenderIdentification ]

Hi Alexander,

On Mon, Jul 31, 2023 at 01:11:35PM +, Stephan, Alexander wrote:
> Dear HAProxy-maintainers,
>
> As proposed by my colleague Christian Menges in [1], I've implemented 
> support for fetching arbitrary TLV values for PROXY protocol V2 via a sample 
> fetch.

I'm afraid I don't remember seeing this message, and it was left without 
response, so it fell through the cracks, that's not cool for your colleague.

> It can be used by calling 'fc_pp_tlv' with the numerical value of the 
> desired TLV type. This also fixes issue [2].

Indeed.

> Some design considerations for my approach:
> o *Do not break the existing API*: Keep methods for authority and 
> unique ID, but refactor them internally with the updated, generified logic.

That's indeed preferable :-)

> o *Keep existing behavior and performance*: Pools for authority and 
> unique ID are still used. Already implemented parsing logic is still 
> applied. All information about a TLV payload that is already should be 
> used for validation.

OK.

> o *Small memory footprint*: As there might be up to 50k connections, 
> an array or hash map is too memory intensive. Therefore, a simple list 
> is used. It is the best choice here in my opinion, as there are 
> typically only a handful of TLVs.

I agree. At first I thought "Hmmm lists?" but we're speaking about very few 
items here, less than a handfull in general, so yes, I agree that the list 
might probably be the cheapest option.

> Additionally, memory pooling is used where possible. When TLV values 
> become too large there is a fallback to 'malloc'.

At first glance I like this approach. We can see in field how it deals with the 
traffic, but it should be OK. However I'm noticing that you put a limit to 127, 
which is a bit unfortunate, because we switched the sockaddr_storage to a pool 
a few years ago, and they're of size 128, so for one extra byte you could pick 
items from that shared pool.

> Note that I choose to not overwrite existing TLVs in the list. This 
> way, we return always the first found but would allow duplicate 
> entries. This would be relevant when there are duplicate TLVs.

Good point, I hadn't thought about this. And this is desirable because our ACLs 
can iterate over multiple instances of a sample. It seems that the current 
function smp_fetch_fc_pp_tlv() doesn't support it by the way, and will only 
return the first one. Please have a look at
smp_fetch_ssl_hello_alpn() or more generally some HTTP sample fetches that set 
SMP_F_NOT_LAST. This tells the ACL engine that if the last returned entry does 
not match, it can come back with the same context to retrieve the next one, and 
so on. Once you reach the end, you just drop that flag and the

Re: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values

2023-08-10 Thread Willy Tarreau

Hi Alexander,

On Mon, Jul 31, 2023 at 01:11:35PM +, Stephan, Alexander wrote:
> Dear HAProxy-maintainers,
> 
> As proposed by my colleague Christian Menges in [1], I've implemented support
> for fetching arbitrary TLV values for PROXY protocol V2 via a sample fetch.

I'm afraid I don't remember seeing this message, and it was left without
response, so it fell through the cracks, that's not cool for your colleague.

> It can be used by calling 'fc_pp_tlv' with the numerical value of the desired
> TLV type. This also fixes issue [2].

Indeed.

> Some design considerations for my approach:
> o *Do not break the existing API*: Keep methods for authority and unique ID,
> but refactor them internally with the updated, generified logic.

That's indeed preferable :-)

> o *Keep existing behavior and performance*: Pools for authority and unique ID
> are still used. Already implemented parsing logic is still applied. All
> information about a TLV payload that is already should be used for
> validation.

OK.

> o *Small memory footprint*: As there might be up to 50k connections, an array
> or hash map is too memory intensive. Therefore, a simple list is used. It is
> the best choice here in my opinion, as there are typically only a handful of
> TLVs.

I agree. At first I thought "Hmmm lists?" but we're speaking about very
few items here, less than a handfull in general, so yes, I agree that
the list might probably be the cheapest option.

> Additionally, memory pooling is used where possible. When TLV values
> become too large there is a fallback to 'malloc'.

At first glance I like this approach. We can see in field how it deals
with the traffic, but it should be OK. However I'm noticing that you put
a limit to 127, which is a bit unfortunate, because we switched the
sockaddr_storage to a pool a few years ago, and they're of size 128, so
for one extra byte you could pick items from that shared pool.

> Note that I choose to not overwrite existing TLVs in the list. This way, we
> return always the first found but would allow duplicate entries. This would
> be relevant when there are duplicate TLVs.

Good point, I hadn't thought about this. And this is desirable because
our ACLs can iterate over multiple instances of a sample. It seems that
the current function smp_fetch_fc_pp_tlv() doesn't support it by the way,
and will only return the first one. Please have a look at
smp_fetch_ssl_hello_alpn() or more generally some HTTP sample fetches that
set SMP_F_NOT_LAST. This tells the ACL engine that if the last returned
entry does not match, it can come back with the same context to retrieve
the next one, and so on. Once you reach the end, you just drop that flag
and the ACL engine has its final status. In your context you would store
a retry point (maybe even the list's pointer). There's even a
list_for_each_entry_from() to restart from a known point.

> Besides, I used 'strtoul' for the argument conversion to be consistent with
> other fetches that already use 'strtoul'.
> If 'strtoul' is too slow for this use case, I am not opposed to reverting it
> to the more efficient HAProxy helper.

That's not the right way to do it. When you declare a sample fetch
function, you can also declare a function that will check its arguments.
And that function can replace these args. It's done quite a bit to turn a
string to int after verifying a range for example. Please have a look at
sample_conv_mqtt_field_value_check() to see how that's used. This could
even allow you to implement symbolic names as aliases for know integer
values.

> *Important*: I will add the documentation after the code review, when the
> design is finalized.

Yeah I understand :-)

Now the patch below:

> diff --git a/include/haproxy/connection-t.h b/include/haproxy/connection-t.h
> index eb02bbcf2..a7ebed5ec 100644
> --- a/include/haproxy/connection-t.h
> +++ b/include/haproxy/connection-t.h
> @@ -501,6 +501,17 @@ struct conn_hash_params {
>   struct sockaddr_storage *dst_addr;
>  };
>  
> +/*
> + * This structure describes an TLV entry consisting of its type
> + * and corresponding payload. This can be used to construct a list
> + * from which arbitrary TLV payloads can be fetched.
> + */
> +struct conn_tlv_list {
> +struct list list;
> +struct ist value;
> +unsigned char type;
> +};
> +
>  /* This structure describes a connection with its methods and data.
>   * A connection may be performed to proxy or server via a local or remote
>   * socket, and can also be made to an internal applet. It can support
> @@ -534,10 +545,9 @@ struct connection {
>  
>   /* third cache line and beyond */
>   void (*destroy_cb)(struct connection *conn);  /* callback to notify of 
> imminent death of the connection */
> - struct sockaddr_storage *src; /* source address

Re: [PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values

2023-08-05 Thread Willy Tarreau

Hi Alexander,

On Mon, Jul 31, 2023 at 01:11:35PM +, Stephan, Alexander wrote:
> Dear HAProxy-maintainers,
> 
> As proposed by my colleague Christian Menges in [1], I've implemented support
> for fetching arbitrary TLV values for PROXY protocol V2 via a sample fetch.
> It can be used by calling 'fc_pp_tlv' with the numerical value of the desired
> TLV type. This also fixes issue [2].
(...)

Still not had the time to review your work in details, though at first
glance I suspect it looks fine. I hope to be able to review it next week
but at least wanted to ack receipt.

Thanks,
Willy

[PATCH] MEDIUM: sample: Implement sample fetch for arbitrary PROXY protocol v2 TLV values

2023-07-31 Thread Stephan, Alexander

Dear HAProxy-maintainers,

As proposed by my colleague Christian Menges in [1], I’ve implemented support 
for fetching arbitrary TLV values for PROXY protocol V2 via a sample fetch. It 
can be used by calling ‘fc_pp_tlv’ with the numerical value of the desired TLV 
type. This also fixes issue [2].

Some design considerations for my approach:
• *Do not break the existing API*: Keep methods for authority and unique ID, 
but refactor them internally with the updated, generified logic.
• *Keep existing behavior and performance*: Pools for authority and unique ID 
are still used. Already implemented parsing logic is still applied. All 
information about a TLV payload that is already should be used for validation.
• *Small memory footprint*: As there might be up to 50k connections, an array 
or hash map is too memory intensive. Therefore, a simple list is used. It is 
the best choice here in my opinion, as there are typically only a handful of 
TLVs. Additionally, memory pooling is used where possible. When TLV values 
become too large there is a fallback to ‘malloc’.

Note that I choose to not overwrite existing TLVs in the list. This way, we 
return always the first found but would allow duplicate entries. This would be 
relevant when there are duplicate TLVs.
Besides, I used ‘strtoul’ for the argument conversion to be consistent with 
other fetches that already use ‘strtoul’.
If ‘strtoul’ is too slow for this use case, I am not opposed to reverting it to 
the more efficient HAProxy helper.

*Important*: I will add the documentation after the code review, when the 
design is finalized.

This email address is not subscribed to the list, please CC it when replying.

Anyway, I would love to hear your feedback on this!

Best Regards,
Alexander Stephan
SAP SE Germany

PS: This is only the front-end part of my implementation, I will follow up with 
a backend implementation that allows to send
user defined TLVs. I already implemented it, since SAP needs it any case. 
However, we are also very interested in contributing it.

[1]: https://www.mail-archive.com/haproxy@formilux.org/msg43381.html
[2]: https://github.com/haproxy/haproxy/issues/1947



0001-MEDIUM-sample-Implement-sample-fetch-for-arbitrary-P.patch
Description:  0001-MEDIUM-sample-Implement-sample-fetch-for-arbitrary-P.patch

RE: Interest in HA Proxy from Sonicwall

2023-04-05 Thread Kenny Lederman

Hi Team,

You can disregard as Jonathan Purcell 
jpurc...@haproxy.com<mailto:jpurc...@haproxy.com> has already reached out to me 
on this request.

Thank you.

Kenny Lederman
Enterprise Account Manager
(206) 455-6488 - Office
(847) 932-9771 - Cell
kenny.leder...@softchoice.com<mailto:kenny.leder...@softchoice.com>
[cid:image001.gif@01D967B5.F0910E40]<https://www.softchoice.com/>
[Softchoice]<https://www.softchoice.com/>
415 1st Avenue North, Suite 300
Seattle, WA  98109

From: Илья Шипицин 
Sent: Wednesday, April 5, 2023 11:56 AM
To: Aleksandar Lazic 
Cc: Kenny Lederman ; haproxy@formilux.org
Subject: Re: Interest in HA Proxy from Sonicwall

External message. Do not click links or open attachments unless you recognize 
the source. Message externe. Ne cliquez sur aucun lien et n’ouvrez aucune pièce 
jointe à moins d’en connaître la provenance.

ср, 5 апр. 2023 г. в 20:18, Aleksandar Lazic 
mailto:al-hapr...@none.at>>:
Hi Kenny.

On 05.04.23 20:04, Kenny Lederman wrote:
> Hi team,
>
> Do you have an account rep assigned to Sonicwall that could help me with
> getting a POC set up?

This is the Open Source Mailing list, if you want to get in touch with
the Company behind HAProxy please use this.

original intention was not clear :) maybe Kenny is looking for open source 
individuals to hire them in purpose.

otherwise, yes, https://www.haproxy.com<https://www.haproxy.com/> is proper way 
to contact sales/commercial/whatever.

https://www.haproxy.com/contact-us/

Of course can you setup the Open Source HAProxy by your team, the
documentation is hosted at this URL.

http://docs.haproxy.org/

> Thank you,
>
> Kenny Lederman

Best Regards
Alex

> Enterprise Account Manager
>
> (206) 455-6488 - Office
>
> (847) 932-9771 - Cell
>
> kenny.leder...@softchoice.com<mailto:kenny.leder...@softchoice.com> 
> <mailto:kenny.leder...@softchoice.com<mailto:kenny.leder...@softchoice.com>>
>
> <https://www.softchoice.com/>
>
>
>
> Softchoice <https://www.softchoice.com/>
>
>
>
> 415 1st Avenue North, Suite 300
> Seattle, WA  98109
>
>
>
> Manage Subscription
> <https://tech.softchoice.com/subscription-center>Unsubscribe
> <https://tech.softchoice.com/subscription-center>Privacy
> <https://www.softchoice.com/about-softchoice/help/privacy>
>

Manage Subscription<https://tech.softchoice.com/subscription-center>   
Unsubscribe<https://tech.softchoice.com/subscription-center>  
Privacy<https://www.softchoice.com/about-softchoice/help/privacy>

Re: Interest in HA Proxy from Sonicwall

2023-04-05 Thread Илья Шипицин

ср, 5 апр. 2023 г. в 20:18, Aleksandar Lazic :

> Hi Kenny.
>
> On 05.04.23 20:04, Kenny Lederman wrote:
> > Hi team,
> >
> > Do you have an account rep assigned to Sonicwall that could help me with
> > getting a POC set up?
>
> This is the Open Source Mailing list, if you want to get in touch with
> the Company behind HAProxy please use this.
>

original intention was not clear :) maybe Kenny is looking for open source
individuals to hire them in purpose.

otherwise, yes, https://www.haproxy.com is proper way to contact
sales/commercial/whatever.


>
> https://www.haproxy.com/contact-us/
>
> Of course can you setup the Open Source HAProxy by your team, the
> documentation is hosted at this URL.
>
> http://docs.haproxy.org/
>
> > Thank you,
> >
> > Kenny Lederman
>
> Best Regards
> Alex
>
> > Enterprise Account Manager
> >
> > (206) 455-6488 - Office
> >
> > (847) 932-9771 - Cell
> >
> > kenny.leder...@softchoice.com 
> >
> > 
> >
> >
> >
> > Softchoice 
> >
> >
> >
> > 415 1st Avenue North, Suite 300
> > Seattle, WA  98109
> >
> >
> >
> > Manage Subscription
> > Unsubscribe
> > Privacy
> > 
> >
>
>

Re: Interest in HA Proxy from Sonicwall

2023-04-05 Thread Aleksandar Lazic


Hi Kenny.

On 05.04.23 20:04, Kenny Lederman wrote:

Hi team,

Do you have an account rep assigned to Sonicwall that could help me with 
getting a POC set up?


This is the Open Source Mailing list, if you want to get in touch with 
the Company behind HAProxy please use this.


https://www.haproxy.com/contact-us/

Of course can you setup the Open Source HAProxy by your team, the 
documentation is hosted at this URL.


http://docs.haproxy.org/


Thank you,

Kenny Lederman


Best Regards
Alex


Enterprise Account Manager

(206) 455-6488 - Office

(847) 932-9771 - Cell

kenny.leder...@softchoice.com 





Softchoice 



415 1st Avenue North, Suite 300
Seattle, WA  98109



Manage Subscription 
Unsubscribe 
Privacy

Interest in HA Proxy from Sonicwall

2023-04-05 Thread Kenny Lederman

Hi team,

Do you have an account rep assigned to Sonicwall that could help me with 
getting a POC set up?

Thank you,

Kenny Lederman
Enterprise Account Manager
(206) 455-6488 - Office
(847) 932-9771 - Cell
kenny.leder...@softchoice.com
[cid:image001.gif@01D967AE.74FCB830]
[Softchoice]
415 1st Avenue North, Suite 300
Seattle, WA  98109



Manage Subscription   
Unsubscribe  
Privacy

Support for arbitrary Proxy Protocol v2 TLVs

2023-03-31 Thread Menges, Christian Norbert

Dear HAProxy-maintainers,

We would like to follow up on the patch to make arbitrary proxy protocol TLV 
entries accessible (submitted in 
https://www.mail-archive.com/haproxy@formilux.org/msg43082.html). While the 
goal of having the TLV entries available for custom processing, e.g. in the 
backend, is clear, we are not sure whether capture is the right mechanism to 
realize this behaviour. Capture is often used to make information visible in 
the logs, but not to make information available in the backend. In addition, 
the index-based interface to retrieve captured values is not suitable for TLVs, 
since TLVs have a key-value semantic and using the key for retrieval would 
enhance readability and is less error-prone. A possible alternative solution 
would be to store the TLV block in the same way as the authority 
(PP2_TYPE_AUTHORITY). This could result in a key based interface (e.g. 
req.pp2.tlv()).

Another aspect which needs discussion are subtypes. Subtypes according to the 
proxy protocol specification can be part of the TLV type. However, the term 
subtype in the submitted patch has a different meaning and is part of the 
value. As such it is vendor specific and has no standardized format. We believe 
that the processing of anything contained in the value should be up to the user 
and not to HAProxy. E.g. the user could evaluate the vendor-specific subtype by 
means of ACL rules.

Before implementing an updated version of the patch, we would like to get your 
opinion on the above mentioned open questions. We are aware of some issues in 
the first patch which are violating your guidelines, which we will also address 
in an updated version.

Best regards,
Christian Menges
SAP SE Germany

Re: Transparent proxy issue on FreeBSD

2023-03-07 Thread Rainer Duffner




> Am 07.03.2023 um 18:26 schrieb Marc West :
> 
> On 2023-03-07 08:09:04, Rainer Duffner wrote:
>> I admit I only toyed with TP, so I really don???t know what I???m doing 
>> there, but:
>> 
>> Have you tried to just use pfSense for this? The developer of the package 
>> (https://github.com/PiBa-NL) seemed to be active here, but I haven???t seen 
>> anything from him since 2020, so I wonder if he has moved on.
>> 
>> My co-workers use OPNSense for this purpose - and on VMWare, they insist 
>> that only em(4) NICs work.
>> 
>> 
>> If you don???t find his email-address, I can mail it to you.
> 
> Thanks for the suggestion. I haven't tried HAProxy on pfSense but the
> working transparent config and related ipfw fwd rules we have did come
> from PiBa-NL [1].


Ah, ok.

Either ask on the freebsd-forum or the mailing-list - or try with 
OPNSense/pfSense and if the problem persists, you might get more response on 
the forums there.

pf and ipfw are very specialized parts of the kernel and very few developers 
want to touch it, AFAIK.


> Everything does function perfectly until a brief
> period with production traffic and something happens to cause the tproxy
> bind errors and request failures to start. I'm just not sure what is
> going wrong or how to debug further.
> 
> [1] https://www.mail-archive.com/haproxy@formilux.org/msg09923.html
>

Re: Transparent proxy issue on FreeBSD

2023-03-07 Thread Marc West

On 2023-03-07 08:09:04, Rainer Duffner wrote:
> I admit I only toyed with TP, so I really don???t know what I???m doing 
> there, but:
> 
> Have you tried to just use pfSense for this? The developer of the package 
> (https://github.com/PiBa-NL) seemed to be active here, but I haven???t seen 
> anything from him since 2020, so I wonder if he has moved on.
> 
> My co-workers use OPNSense for this purpose - and on VMWare, they insist that 
> only em(4) NICs work.
> 
> 
> If you don???t find his email-address, I can mail it to you.

Thanks for the suggestion. I haven't tried HAProxy on pfSense but the
working transparent config and related ipfw fwd rules we have did come
from PiBa-NL [1]. Everything does function perfectly until a brief
period with production traffic and something happens to cause the tproxy
bind errors and request failures to start. I'm just not sure what is
going wrong or how to debug further.

[1] https://www.mail-archive.com/haproxy@formilux.org/msg09923.html

Re: Transparent proxy issue on FreeBSD

2023-03-07 Thread Rainer Duffner




> Am 07.03.2023 um 08:46 schrieb Marc West :
> 
> 
> 
> Any other thoughts to look at or data that would be helpful to collect?
> 


I admit I only toyed with TP, so I really don’t know what I’m doing there, but:

Have you tried to just use pfSense for this? The developer of the package 
(https://github.com/PiBa-NL) seemed to be active here, but I haven’t seen 
anything from him since 2020, so I wonder if he has moved on.

My co-workers use OPNSense for this purpose - and on VMWare, they insist that 
only em(4) NICs work.


If you don’t find his email-address, I can mail it to you.

Re: Transparent proxy issue on FreeBSD

2023-03-06 Thread Marc West

Hi Stefan and thanks for your replies. 

(Sorry for the late reply and replying to my own mail, I don't seem to
be receiving messages from the list after confirming the subscription
twice and noticed your replies when checking the archives.)

> when I understand you correct then you have forwarding enabled to that
> ports on pf.
> 
> I had a similar issue on pfsense. The solution was to disable the
> forwarding to that port.

PF isn't doing anything special with the public IPs/ports that HAProxy
binds to, only allowing that traffic. PF does do outbound NAT for
internal servers to reach the Internet like so:

table  const { 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16 }
nat pass on $pub_vlan from 10.10.15.0/24 to ! -> 1.2.3.4

Would a firewall be able to cause the HAProxy tproxy bind errors for
some (but not all) transparent connections? I believe firewalls could
block connections but shouldn't prevent the actual haproxy bind from
succeeding (?). I read through the code and see where the tproxy bind
error is being hit but unsure what is causing it to succeed sometimes
and fail others.

It doesn't seem like it would be an issue allocating or exhausting ports
since the original client IP+port is being reused with "usesrc client"
and there shouldn't be conflicts there. On FreeBSD there are no similar
sysctls to Linux's net.ipv4.ip_nonlocal_bind, and transparent does work
some of the time with my existing config until it starts failing.

> one another:
> 
> source ipv4@ usesrc clientip

I have separate backends/frontends for IPv4 and IPv6 with "source
0.0.0.0 usesrc client" in defaults (also tried "clientip"), which in my
understanding should do the right thing for both v4 and v6 respectively.
Would there be something different about using ipv4@ here?

Any other thoughts to look at or data that would be helpful to collect?

Re: Transparent proxy issue on FreeBSD

2023-02-23 Thread Stefan Fuhrmann

2 TLSv1.3
Built with Lua version : Lua 5.3.6
Built with the Prometheus exporter as a service
Support for malloc_trim() is enabled.
Built with zlib version : 1.2.12
Running on zlib version : 1.2.12
Compression algorithms supported : identity("identity"), deflate("deflate"), 
raw-deflate("deflate"), gzip("gzip")
Built with transparent proxy support using: IP_BINDANY IPV6_BINDANY
Built with PCRE2 version : 10.40 2022-04-14
PCRE2 library supports JIT : yes
Encrypted password support via crypt(3): yes
Built with clang compiler version 13.0.0 (g...@github.com:llvm/llvm-project.git 
 llvmorg-13.0.0-0-gd7b669b3a303)

Available polling systems :
  kqueue : pref=300,  test result OK
poll : pref=200,  test result OK
  select : pref=150,  test result OK
Total: 3 (3 usable), will use kqueue.

Available multiplexer protocols :
(protocols marked as  cannot be specified using 'proto' keyword)
  h2 : mode=HTTP  side=FE|BE  mux=H2flags=HTX|HOL_RISK|NO_UPG
fcgi : mode=HTTP  side=BE mux=FCGI  flags=HTX|HOL_RISK|NO_UPG
  h1 : mode=HTTP  side=FE|BE  mux=H1flags=HTX|NO_UPG
: mode=HTTP  side=FE|BE  mux=H1flags=HTX
none : mode=TCP   side=FE|BE  mux=PASS  flags=NO_UPG
: mode=TCP   side=FE|BE  mux=PASS  flags=

Available services : prometheus-exporter
Available filters :
[CACHE] cache
[COMP] compression
[FCGI] fcgi-app
[SPOE] spoe
[TRACE] trace
$

Re: Transparent proxy issue on FreeBSD

2023-02-23 Thread Stefan Fuhrmann

y 2022
Running on OpenSSL version : OpenSSL 1.1.1o-freebsd  3 May 2022
OpenSSL library supports TLS extensions : yes
OpenSSL library supports SNI : yes
OpenSSL library supports : TLSv1.0 TLSv1.1 TLSv1.2 TLSv1.3
Built with Lua version : Lua 5.3.6
Built with the Prometheus exporter as a service
Support for malloc_trim() is enabled.
Built with zlib version : 1.2.12
Running on zlib version : 1.2.12
Compression algorithms supported : identity("identity"), deflate("deflate"), 
raw-deflate("deflate"), gzip("gzip")
Built with transparent proxy support using: IP_BINDANY IPV6_BINDANY
Built with PCRE2 version : 10.40 2022-04-14
PCRE2 library supports JIT : yes
Encrypted password support via crypt(3): yes
Built with clang compiler version 13.0.0 (g...@github.com:llvm/llvm-project.git 
llvmorg-13.0.0-0-gd7b669b3a303)

Available polling systems :
  kqueue : pref=300,  test result OK
poll : pref=200,  test result OK
  select : pref=150,  test result OK
Total: 3 (3 usable), will use kqueue.

Available multiplexer protocols :
(protocols marked as  cannot be specified using 'proto' keyword)
  h2 : mode=HTTP  side=FE|BE  mux=H2flags=HTX|HOL_RISK|NO_UPG
fcgi : mode=HTTP  side=BE mux=FCGI  flags=HTX|HOL_RISK|NO_UPG
  h1 : mode=HTTP  side=FE|BE  mux=H1flags=HTX|NO_UPG
: mode=HTTP  side=FE|BE  mux=H1flags=HTX
none : mode=TCP   side=FE|BE  mux=PASS  flags=NO_UPG
: mode=TCP   side=FE|BE  mux=PASS  flags=

Available services : prometheus-exporter
Available filters :
[CACHE] cache
[COMP] compression
[FCGI] fcgi-app
[SPOE] spoe
[TRACE] trace
$

Transparent proxy issue on FreeBSD

2023-02-17 Thread Marc West

.
Built with zlib version : 1.2.12
Running on zlib version : 1.2.12
Compression algorithms supported : identity("identity"), deflate("deflate"), 
raw-deflate("deflate"), gzip("gzip")
Built with transparent proxy support using: IP_BINDANY IPV6_BINDANY
Built with PCRE2 version : 10.40 2022-04-14
PCRE2 library supports JIT : yes
Encrypted password support via crypt(3): yes
Built with clang compiler version 13.0.0 (g...@github.com:llvm/llvm-project.git 
llvmorg-13.0.0-0-gd7b669b3a303)

Available polling systems :
 kqueue : pref=300,  test result OK
   poll : pref=200,  test result OK
 select : pref=150,  test result OK
Total: 3 (3 usable), will use kqueue.

Available multiplexer protocols :
(protocols marked as  cannot be specified using 'proto' keyword)
 h2 : mode=HTTP  side=FE|BE  mux=H2flags=HTX|HOL_RISK|NO_UPG
   fcgi : mode=HTTP  side=BE mux=FCGI  flags=HTX|HOL_RISK|NO_UPG
 h1 : mode=HTTP  side=FE|BE  mux=H1flags=HTX|NO_UPG
   : mode=HTTP  side=FE|BE  mux=H1flags=HTX
   none : mode=TCP   side=FE|BE  mux=PASS  flags=NO_UPG
   : mode=TCP   side=FE|BE  mux=PASS  flags=

Available services : prometheus-exporter
Available filters :
[CACHE] cache
[COMP] compression
[FCGI] fcgi-app
[SPOE] spoe
[TRACE] trace
$

Re: Support arbitrary PROXY protocol v2 TLVs as samples

2023-02-16 Thread Bitsch, Johannes (external - Project)

Hi Willy,

any chance you already found some time to give this a review?
Looking forward to your comments!

Thanks,
Johannes

Re: Support arbitrary PROXY protocol v2 TLVs as samples

2023-01-24 Thread Willy Tarreau

Hi Johannes,

On Wed, Jan 18, 2023 at 10:49:18AM +, Bitsch, Johannes (external - Project) 
wrote:
> Hi again,
> 
> I checked my patch file from a few weeks ago using the recommended
> checkpatch.pl [1] and realized that the indentation was off as well as some
> other small things.
> To make this easier to review, I fixed all the issues mentioned by checkpatch
> (except for editing MAINTAINERS, I don't think we're quite there yet). The
> implementation was not changed. Apologies for not checking this earlier.

No problem, thanks, I'll have a look hopefully this week. Do not hesitate
to ping again if you don't see a comment, as constantly switching between
plenty of issues and reviews makes it easy to just forget some of them.

Thanks,
Willy

Re: Support arbitrary PROXY protocol v2 TLVs as samples

2023-01-18 Thread Bitsch, Johannes (external - Project)

Hi again,

I checked my patch file from a few weeks ago using the recommended 
checkpatch.pl [1] and realized that the indentation was off as well as some 
other small things.
To make this easier to review, I fixed all the issues mentioned by checkpatch 
(except for editing MAINTAINERS, I don't think we're quite there yet). The 
implementation was not changed. Apologies for not checking this earlier.

Please let me know if there's anything else I can do.

Regards,
Johannes

[1] http://www.haproxy.org/coding-style.html



0001-wip-MINOR-proxy_protocol-add-tlv-value-captures.patch
Description:  0001-wip-MINOR-proxy_protocol-add-tlv-value-captures.patch

Re: proxy

2023-01-11 Thread Aleksandar Lazic


Hi Adam.

On 12.01.23 01:30, Adam wrote:

Dear Friend
I have a service to broadcast channels and movies over the Internet
by panel iptv
And I have servers that I want to hide the real IP of in order to 
protect them from attacks

It is on the other hand a complaint of abuse
How do you help me with that
I have more than 10 Ubuntu servers
I am waiting for your reply


You can use haproxy for that and there are quite good blog posts about 
protection of backend servers.


https://www.haproxy.com/blog/category/security/

As you have also contacted cont...@haproxy.com you could get a offer for 
the HAProxy Enterprise product 
https://www.haproxy.com/products/haproxy-enterprise/ .


Regards
Alex

proxy

2023-01-11 Thread Adam

Dear Friend
 I have a service to broadcast channels and movies over the Internet
by panel iptv
And I have servers that I want to hide the real IP of in order to protect
them from attacks
It is on the other hand a complaint of abuse
How do you help me with that
I have more than 10 Ubuntu servers
I am waiting for your reply

proxy ip list please?

2023-01-05 Thread SS


Hi


thx for website, but very busy site and i can't find any Ip list for
Iran whatsapp,


can you send me please direct link to download ip list or send me ip
list i am not computer specialist or programmer, i dont know github...


send me pls ip list for Iran people;)


Thx!

KR.

Re: Support arbitrary PROXY protocol v2 TLVs as samples

2023-01-02 Thread Bitsch, Johannes (external - Project)

Hi Willy,

thanks for taking the time!

> I've had a quick look, but it's difficult to have an opinion. The main
> concerns of capturing TLV values during parsing are technical ones, mostly
> the impacts on memory usage. You seem to have worked out a patch already to
> do this, so it would be more convenient to discuss the possibilities around
> your existing patch; it wouldn't make sense to have to redo all the analysis
> you've already done, and if someone would propose a patch doing the same as
> you did, it wouldn't be very kind for your existing work.
>
> Would you mind sharing your patch here for a discussion or review ?

That makes sense. I attached the patch below. It still contains some debugging 
printfs that I added as well as some TODOs in areas I was particularly unsure 
about. The implementation is similar to the existing feature that captures http 
headers.
I also added a small test in reg-tests/tlv-capture/multiple_tlvs.vtc that shows 
how it can be used.

I'd love to get some feedback!

Regards,
Johannes






0001-wip-MINOR-proxy_protocol-add-tlv-value-captures.patch
Description:  0001-wip-MINOR-proxy_protocol-add-tlv-value-captures.patch

Re: Ha proxy frontend

2022-12-28 Thread Willy Tarreau

On Thu, Dec 29, 2022 at 11:33:03AM +0500, Ghufran Shahzad wrote:
> Yes, sure, I make 2 azure vms, and install mysql server and use load
> balancer , mysql percona clusters , then i install ha proxy on both vms but
> when i access them it is not working, can you please give me a solution?

You realize that there is almost no info here ? What's your configuration,
which ip/ports are each service bound to and do the correspond to what is
in the config, what do you see in your logs, are the backend servers checked
or not, and if so are they reported up ? What does "is not working" mean in
your situation, does it mean you get a "Trying..." which would indicate that
you are connecting to the wrong address, or "Connection refused" that means
that you're probably connecting to the wrong port, or does the connection
establish and nothing happens, which could mean that the connection with the
backend server isn't in a good state ?

You will hardly get more help if you're not doing at least some minimal
homework, particularly on the elements we cannot guess for you.

Hoping this helps,
Willy

PS: please keep the list in Cc when you respond.
PPS: please avoid top-posting, it generally indicates that responses
 are not read, and makes it more annoying for others to respond.

Re: Ha proxy frontend

2022-12-28 Thread Willy Tarreau

On Thu, Dec 29, 2022 at 11:26:43AM +0500, Ghufran Shahzad wrote:
> how we can access frontend ip on ha proxy? kindly give me detailed
> solution. thanks

Could you please precise your question ?

Willy

Ha proxy frontend

2022-12-28 Thread Ghufran Shahzad

how we can access frontend ip on ha proxy? kindly give me detailed
solution. thanks

Re: Support arbitrary PROXY protocol v2 TLVs as samples

2022-12-25 Thread Willy Tarreau

Hi Johannes,

On Fri, Dec 23, 2022 at 02:08:09PM +, Bitsch, Johannes (external - Project) 
wrote:
> Hi all,
> 
> I created a feature request on github about supporting arbitrary PROXY
> protocol v2 TLVs in haproxy a few weeks ago[1].
> 
> Since I haven't received any feedback or reactions on it so far, I was
> wondering if this was the right place to discuss something like this.
> Is there a better place to have a discussion about it, e.g. on this mailing
> list or somewhere else? If you could point me to the best way to do this, I'd
> be happy to switch.

Well, I'd say that the mailing list is more suitable to design discussions
and the issue tracker is best suited to track what remains to be done. The
issue tracker by nature is used as a high frequency context-switch, and is
essentially used to ask for more info about a reported bug, but it is also
used as a todo list. It just happens that you started the discussion in the
middle of the release cycle and that unfortunately that's the worst moment
to get anyone's attention, and it's not guaranteed that you would have had
more responses on the ML at this period :-)

> I'll avoid duplicating the request here for now but will happily copy/paste
> it in here if that's easier for someone.

I've had a quick look, but it's difficult to have an opinion. The main
concerns of capturing TLV values during parsing are technical ones, mostly
the impacts on memory usage. You seem to have worked out a patch already to
do this, so it would be more convenient to discuss the possibilities around
your existing patch; it wouldn't make sense to have to redo all the analysis
you've already done, and if someone would propose a patch doing the same as
you did, it wouldn't be very kind for your existing work.

Would you mind sharing your patch here for a discussion or review ?

Thanks,
Willy

Support arbitrary PROXY protocol v2 TLVs as samples

2022-12-23 Thread Bitsch, Johannes (external - Project)

Hi all,

I created a feature request on github about supporting arbitrary PROXY protocol 
v2 TLVs in haproxy a few weeks ago[1].

Since I haven't received any feedback or reactions on it so far, I was 
wondering if this was the right place to discuss something like this.
Is there a better place to have a discussion about it, e.g. on this mailing 
list or somewhere else? If you could point me to the best way to do this, I'd 
be happy to switch.

I'll avoid duplicating the request here for now but will happily copy/paste it 
in here if that’s easier for someone.

Thanks & regards,
Johannes 

[1] https://github.com/haproxy/haproxy/issues/1947

Re: Haproxy send-proxy probes error

2022-11-23 Thread Aleksandar Lazic

Hi.

There is already a bug entry in apache bz from 2019 about that message.

https://bz.apache.org/bugzilla/show_bug.cgi?id=63893

Regards
Alex

23.11.2022 21:36:26 Marcello Lorenzi :

> Hi All,
> we use haproxy 2.2.17-dd94a25 in our development environment and we configure 
> a backend with proxy protocol v2 to permit the source IP forwarding to a TLS 
> backend server. All the configuration works fine but we notice this error 
> reported on backend Apache error logs:
> 
> AH03507: RemoteIPProxyProtocol: unsupported command 20
> 
> We configure the options check-send-proxy on backend probes but the issue 
> persists. 
> 
> Is it possible to remove this persistent error?
> 
> Thanks,
> Marcello

Haproxy send-proxy probes error

2022-11-23 Thread Marcello Lorenzi

Hi All,
we use haproxy 2.2.17-dd94a25 in our development environment and we
configure a backend with proxy protocol v2 to permit the source IP
forwarding to a TLS backend server. All the configuration works fine but we
notice this error reported on backend Apache error logs:

AH03507: RemoteIPProxyProtocol: unsupported command 20

We configure the options check-send-proxy on backend probes but the issue
persists.

Is it possible to remove this persistent error?

Thanks,
Marcello

Re: HA Proxy License

2022-10-07 Thread Aleksandar Lazic

Hi John.

I suggest to get in touch whith HAProxy company via this form.

https://www.haproxy.com/contact-us/

best regards
alex

07.10.2022 17:55:42 John Bowling (CE CEN) :

> Hello,
> 
> What are the costs for the license or is there a subscription for license?
> 
> *John L. Bowling (JB)*
> 
> Senior Team Leader
> 
> *IES – Network Engineering & Security (NES)*
> 
> *Network Operational Readiness (NOC)*
> 
> Whole Foods Market – Global Support (CEN)
> 
> An Amazon Company
> 
> 1011 W 5th  Street, 4th floor
> 
> Austin, Texas USA 78703
> 
> Mobile: +1-512-221-3780
> 
> Desk: +1-512.542.0797
> 
> Email: john.bowl...@wholefoods.com
> 
> www.wholefoodsmarket.com[http://www.wholefoodsmarket.com/]
> 
> Four principles: customer obsession rather than competitor focus, passion for 
> invention, commitment to operational excellence, and long-term thinking
> 
>  
> 
> For WFM technical support please call Global Help Desk at 1-877-923-4263  
>  
> 
>  Monday-Friday 6:00am-9:00pm CST Sat & Sun: 8:00am-4:00pm
> 
> For service request, open up WFM Internal ticket in OrchardNow 
> OrchardNow[https://wfmprod.service-now.com/nav_to.do?uri=%2Fhome.do%3F]
> 
>  
> 
> This email contains proprietary and confidential material for the sole use of 
> the intended recipient. Any review, use, distribution or disclosure by others 
> without the permission of the sender is strictly prohibited.  If you are not 
> the intended recipient (or authorized to receive for the recipient), please 
> contact the sender by reply email and delete all copies of this message. 
> Thank you.
>

HA Proxy License

2022-10-07 Thread John Bowling (CE CEN)

Hello,

What are the costs for the license or is there a subscription for license?

John L. Bowling (JB)
Senior Team Leader
IES - Network Engineering & Security (NES)
Network Operational Readiness (NOC)
Whole Foods Market - Global Support (CEN)
An Amazon Company
1011 W 5th  Street, 4th floor
Austin, Texas USA 78703
Mobile: +1-512-221-3780
Desk: +1-512.542.0797
Email: john.bowl...@wholefoods.com
www.wholefoodsmarket.com
Four principles: customer obsession rather than competitor focus, passion for 
invention, commitment to operational excellence, and long-term thinking

For WFM technical support please call Global Help Desk at 1-877-923-4263
 Monday-Friday 6:00am-9:00pm CST Sat & Sun: 8:00am-4:00pm
For service request, open up WFM Internal ticket in OrchardNow 
OrchardNow

This email contains proprietary and confidential material for the sole use of 
the intended recipient. Any review, use, distribution or disclosure by others 
without the permission of the sender is strictly prohibited.  If you are not 
the intended recipient (or authorized to receive for the recipient), please 
contact the sender by reply email and delete all copies of this message. Thank 
you.

Re: [PATCH 0/6] 'ist'ify members of struct proxy

2022-03-15 Thread Tim Düsterhus


Willy,

On 3/15/22 08:26, Willy Tarreau wrote:

Or perhaps you could ask and include
me in Cc, at least people already know you. I'll be happy to further improve
the existing Coccinelle patches and to further 'ist'ify the codebase, but
would need some handholding to get me started.


Note that beyond changing code, one thing that would be extremely useful
would be to build up a collection of patches to detect certain classes of
bugs. I wrote some scripts that used to match improbable expressions like:

 unlikely(E) < 0

But I can't figure how to make them match anymore, or they match too much
thus for me the behavior has always been a bit random and I never know if
I can trust the absence of a match.


If you send over your existing Coccinelle files I can have a look into 
cleaning them up and then committing them.



Overall this is still a fantastic tool but it requires a significant and
sustained investment to really unleash its power. Unfortunately in a small
project like haproxy most often it takes less time to perform a few changes
by hand than to try to figure a reliable way to express what we want.



At least for the ist.cocci new cases pop up every now and then and the 
rules within that file are all very very simple. So for that the 
investment into writing the patches is worth it I believe.


I think the hard part is not so much "performing the changes by hand", 
but rather "reliably finding the places by hand". Using grep to find 
accesses of '.ptr' for an ist is going to turn up all kinds of unrelated 
stuff.


Best regards
Tim Düsterhus

Re: [PATCH 0/6] 'ist'ify members of struct proxy

2022-03-15 Thread Willy Tarreau

Hi Tim,

On Fri, Mar 11, 2022 at 09:15:48PM +0100, Tim Düsterhus wrote:
> Yeah, I've attempted to look into the Coccinelle patches in the Linux kernel
> sources, but I agree that many of those are very complex :-)
> 
> Do you happen to know where we could ask for assistance with making the
> necessary adjustments to the patches?

Not really. I'm seeing that the doc on the site was updated less than a
year ago, I should probably re-read it (I had only seen a very old one
with a bunch of examples and too few tips):

 https://coccinelle.gitlabpages.inria.fr/website/

There's also a mailing list (cocci at inria.fr) which is quite active,
but it looks like Julia is very responsive on it and I don't want to
bother her with end-user questions when she likely has more important
things to spend her time on.

> Or perhaps you could ask and include
> me in Cc, at least people already know you. I'll be happy to further improve
> the existing Coccinelle patches and to further 'ist'ify the codebase, but
> would need some handholding to get me started.

Note that beyond changing code, one thing that would be extremely useful
would be to build up a collection of patches to detect certain classes of
bugs. I wrote some scripts that used to match improbable expressions like:

unlikely(E) < 0

But I can't figure how to make them match anymore, or they match too much
thus for me the behavior has always been a bit random and I never know if
I can trust the absence of a match.

Overall this is still a fantastic tool but it requires a significant and
sustained investment to really unleash its power. Unfortunately in a small
project like haproxy most often it takes less time to perform a few changes
by hand than to try to figure a reliable way to express what we want.

Willy

Re: [PATCH 0/6] 'ist'ify members of struct proxy

2022-03-11 Thread Tim Düsterhus


Willy,

[Dropping Christopher from Cc]

On 3/9/22 08:11, Willy Tarreau wrote:

As for the second CLEANUP commit: If one of you knows how to fix the Coccinelle
patch to detect that specific pattern, I'd appreciate if you could make the
necessary changes to ist.cocci. Unfortunately my Coccinelle skills are not
good enough.


I've already faced this situation where it didn't find fields of a given
type inside a structure, and I stopped searching when I figured that by
the time I would finally find, I could have replaced 10 times the 3
occurrences I needed. I essentially use Coccinelle as a helpful tool to
save me time, and I can't resolve myself to spend more time trying to
write unmaintainable scripts that will never be reused. A good source of
inspiration are the scripts in the linux kernel, but those are often of
an extreme level of complexity and mix python scripting within the patch,
resulting in me not understanding anything anymore. But some of them are
still human-readable or at least show the syntax hacks you're looking for.


Yeah, I've attempted to look into the Coccinelle patches in the Linux 
kernel sources, but I agree that many of those are very complex :-)


Do you happen to know where we could ask for assistance with making the 
necessary adjustments to the patches? Or perhaps you could ask and 
include me in Cc, at least people already know you. I'll be happy to 
further improve the existing Coccinelle patches and to further 'ist'ify 
the codebase, but would need some handholding to get me started.


Best regards
Tim Düsterhus

Re: [PATCH 0/6] 'ist'ify members of struct proxy

2022-03-08 Thread Willy Tarreau

Hi Tim,

On Sat, Mar 05, 2022 at 12:52:39AM +0100, Tim Duesterhus wrote:
> Willy,
> Christopher,
> 
> find a series that converts a few members of `struct proxy` into ists. All
> of them have already been converted into ists when operating on them, so
> directly storing them as ists makes that code cleaner.

Now applied, thank you for doing this, it's always good to see such
remains of old code diminish over time. If you're interested in chasing
other ones, I'm seeing that the following commands seem to yield likely
candidates:

   $ git grep -B1 '_len;' include

> One drawback is that `struct proxy` grows by 16 bytes. It might or might not
> be necessary to reorder the struct members to keep it efficient.

Sure but 16 bytes for such a huge struct is not much, and indeed there
are a few holes that we may repack later:

  $ pahole -C proxy ./haproxy
/* size: 6944, cachelines: 109, members: 133 */
/* sum members: 6924, holes: 5, sum holes: 20 */
/* paddings: 1, sum paddings: 4 */
/* last cacheline: 32 bytes */

> The `server_id_hdr_name` one is tagged MEDIUM, because that required some
> non-trivial changes in the FCGI implementation. As I've needed to modify
> that code anyway, I've also added two additional CLEANUP commits.

For me these looked good.

> As for the second CLEANUP commit: If one of you knows how to fix the 
> Coccinelle
> patch to detect that specific pattern, I'd appreciate if you could make the
> necessary changes to ist.cocci. Unfortunately my Coccinelle skills are not
> good enough.

I've already faced this situation where it didn't find fields of a given
type inside a structure, and I stopped searching when I figured that by
the time I would finally find, I could have replaced 10 times the 3
occurrences I needed. I essentially use Coccinelle as a helpful tool to
save me time, and I can't resolve myself to spend more time trying to
write unmaintainable scripts that will never be reused. A good source of
inspiration are the scripts in the linux kernel, but those are often of
an extreme level of complexity and mix python scripting within the patch,
resulting in me not understanding anything anymore. But some of them are
still human-readable or at least show the syntax hacks you're looking for.

> I've tested that HAProxy compiles and that the existing reg-tests pass, but
> I didn't specifically test FCGI (and there are no exiting reg-tests for that).

There are indeed no regtest because we don't have an fcgi server either
in vtest nor in haproxy.

> So please carefully check the patches for dumb mistakes.

For me that was OK so I merged them. If we later find a regression, we'll
both be co-responsible :-)

Thanks,
Willy

Re: Bug: No support of mqtt_is_valid and mqtt_field_value for proxy-protocol connection

2022-03-07 Thread Tim Düsterhus


Hi all

On 3/6/22 18:10, Dhruv Jain wrote:

I would request you to share a work around if possible until it is fixed.



As a heads up: There's an issue in the tracker now. So before replying 
you might want to check there first:


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

Best regards
Tim Düsterhus

Re: Bug: No support of mqtt_is_valid and mqtt_field_value for proxy-protocol connection

2022-03-06 Thread Tim Düsterhus


Dhruv,

On 3/6/22 18:10, Dhruv Jain wrote:

In the following mqtt connection flow, mqtt_is_valid and mqtt_field_value
is not working as intended.

Client -> Google Load Balancer(proxy-protocol enabled) -> HAProxy

Currently the connection is rejected if the ACL rule(
https://pastebin.mozilla.org/TESFMj1b#L57) is mentioned in the frontend
config. Also, if this rule is omitted then mqtt_field_value does not work
due to which there is no entry in the stick table. Logs of the HAProxy
config mentioned in the pastebin can be found here
<https://pastebin.mozilla.org/8q0dUseO>.

Ideally, the connection should be established and an entry should be
created with the key as the client_identifier in the stick table.



I'm not seeing an `accept-proxy` anywhere in the configuration, so 
HAProxy is not actually interpreting (and stripping) the PROXY protocol 
header.


Best regards
Tim Düsterhus

Bug: No support of mqtt_is_valid and mqtt_field_value for proxy-protocol connection

2022-03-06 Thread Dhruv Jain

Hi,

In the following mqtt connection flow, mqtt_is_valid and mqtt_field_value
is not working as intended.

Client -> Google Load Balancer(proxy-protocol enabled) -> HAProxy

Currently the connection is rejected if the ACL rule(
https://pastebin.mozilla.org/TESFMj1b#L57) is mentioned in the frontend
config. Also, if this rule is omitted then mqtt_field_value does not work
due to which there is no entry in the stick table. Logs of the HAProxy
config mentioned in the pastebin can be found here
<https://pastebin.mozilla.org/8q0dUseO>.

Ideally, the connection should be established and an entry should be
created with the key as the client_identifier in the stick table.

I would request you to share a work around if possible until it is fixed.

Thank you.
Regards,
Dhruv Jain

[PATCH 0/6] 'ist'ify members of struct proxy

Willy,
Christopher,

find a series that converts a few members of `struct proxy` into ists. All
of them have already been converted into ists when operating on them, so
directly storing them as ists makes that code cleaner.

One drawback is that `struct proxy` grows by 16 bytes. It might or might not
be necessary to reorder the struct members to keep it efficient.

The `server_id_hdr_name` one is tagged MEDIUM, because that required some
non-trivial changes in the FCGI implementation. As I've needed to modify
that code anyway, I've also added two additional CLEANUP commits.

As for the second CLEANUP commit: If one of you knows how to fix the Coccinelle
patch to detect that specific pattern, I'd appreciate if you could make the
necessary changes to ist.cocci. Unfortunately my Coccinelle skills are not
good enough.

I've tested that HAProxy compiles and that the existing reg-tests pass, but
I didn't specifically test FCGI (and there are no exiting reg-tests for that).
So please carefully check the patches for dumb mistakes.

Best regards

Tim Duesterhus (6):
  MINOR: proxy: Store monitor_uri as a `struct ist`
  MINOR: proxy: Store fwdfor_hdr_name as a `struct ist`
  MINOR: proxy: Store orgto_hdr_name as a `struct ist`
  MEDIUM: proxy: Store server_id_hdr_name as a `struct ist`
  CLEANUP: fcgi: Replace memcpy() on ist by istcat()
  CLEANUP: fcgi: Use `istadv()` in `fcgi_strm_send_params`

 include/haproxy/proxy-t.h | 12 --
 src/cfgparse-listen.c | 46 ---
 src/http_ana.c| 11 --
 src/mux_fcgi.c| 23 ++--
 src/mux_h1.c  |  8 +++
 src/mux_h2.c  |  8 +++
 src/proxy.c   | 37 +--
 7 files changed, 62 insertions(+), 83 deletions(-)

-- 
2.35.1

[PATCH 1/6] MINOR: proxy: Store monitor_uri as a `struct ist`

The monitor_uri is already processed as an ist in `http_wait_for_request`, lets
also just store it as such.

see 0643b0e7e ("MINOR: proxy: Make `header_unique_id` a `struct ist`") for a
very similar past commit.
---
 include/haproxy/proxy-t.h |  3 +--
 src/cfgparse-listen.c |  9 +++--
 src/http_ana.c|  5 ++---
 src/proxy.c   | 11 +--
 4 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/include/haproxy/proxy-t.h b/include/haproxy/proxy-t.h
index 421f900e2..13e722fbf 100644
--- a/include/haproxy/proxy-t.h
+++ b/include/haproxy/proxy-t.h
@@ -322,8 +322,7 @@ struct proxy {
int srvtcpka_cnt;   /* The maximum number of 
keepalive probes TCP should send before dropping the connection. (server side) 
*/
int srvtcpka_idle;  /* The time (in seconds) the 
connection needs to remain idle before TCP starts sending keepalive probes. 
(server side) */
int srvtcpka_intvl; /* The time (in seconds) 
between individual keepalive probes. (server side) */
-   int monitor_uri_len;/* length of the string above. 
0 if unused */
-   char *monitor_uri;  /* a special URI to which we 
respond with HTTP/200 OK */
+   struct ist monitor_uri; /* a special URI to which we 
respond with HTTP/200 OK */
struct list mon_fail_cond;  /* list of conditions to fail 
monitoring requests (chained) */
struct {/* WARNING! check 
proxy_reset_timeouts() in proxy.h !!! */
int client; /* client I/O timeout (in 
ticks) */
diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c
index 5deec5e6b..eb58b2eb1 100644
--- a/src/cfgparse-listen.c
+++ b/src/cfgparse-listen.c
@@ -575,13 +575,10 @@ int cfg_parse_listen(const char *file, int linenum, char 
**args, int kwm)
goto out;
}
 
-   free(curproxy->monitor_uri);
-   curproxy->monitor_uri_len = strlen(args[1]);
-   curproxy->monitor_uri = calloc(1, curproxy->monitor_uri_len + 
1);
-   if (!curproxy->monitor_uri)
+   istfree(>monitor_uri);
+   curproxy->monitor_uri = istdup(ist(args[1]));
+   if (!isttest(curproxy->monitor_uri))
goto alloc_error;
-   memcpy(curproxy->monitor_uri, args[1], 
curproxy->monitor_uri_len);
-   curproxy->monitor_uri[curproxy->monitor_uri_len] = '\0';
 
goto out;
}
diff --git a/src/http_ana.c b/src/http_ana.c
index f33eb7790..b60927e52 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -203,9 +203,8 @@ int http_wait_for_request(struct stream *s, struct channel 
*req, int an_bit)
 * used. It is a workaround to let HTTP/2 health-checks work as
 * expected.
 */
-   if (unlikely(sess->fe->monitor_uri_len != 0)) {
-   const struct ist monitor_uri = ist2(sess->fe->monitor_uri,
-   sess->fe->monitor_uri_len);
+   if (unlikely(isttest(sess->fe->monitor_uri))) {
+   const struct ist monitor_uri = sess->fe->monitor_uri;
struct http_uri_parser parser = 
http_uri_parser_init(htx_sl_req_uri(sl));
 
if ((istptr(monitor_uri)[0] == '/' &&
diff --git a/src/proxy.c b/src/proxy.c
index 946fe13d5..e5cf81327 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -156,7 +156,7 @@ void free_proxy(struct proxy *p)
free(p->lbprm.arg_str);
free(p->server_state_file_name);
free(p->capture_name);
-   free(p->monitor_uri);
+   istfree(>monitor_uri);
free(p->rdp_cookie_name);
free(p->invalid_rep);
free(p->invalid_req);
@@ -1270,7 +1270,7 @@ int proxy_cfg_ensure_no_http(struct proxy *curproxy)
ha_warning("cookie will be ignored for %s '%s' (needs 'mode 
http').\n",
   proxy_type_str(curproxy), curproxy->id);
}
-   if (curproxy->monitor_uri != NULL) {
+   if (isttest(curproxy->monitor_uri)) {
ha_warning("monitor-uri will be ignored for %s '%s' (needs 
'mode http').\n",
   proxy_type_str(curproxy), curproxy->id);
}
@@ -1432,7 +1432,7 @@ void proxy_free_defaults(struct proxy *defproxy)
ha_free(>cookie_attrs);
ha_free(>lbprm.arg_str);
ha_free(>capture_name);
-   ha_free(>monitor_uri);
+   istfree(>monitor_uri);
ha_free(>defbe.name);
    ha_free(>conn_src.iface_name);
ha_free(>fwdfor_hdr_name); defproxy->fwdfor_hdr_len = 0;
@@ -1707,9 +1707,8 @@ static int proxy_defproxy_cpy(struct proxy *curproxy, 
const struct proxy *defp

[PATCH 4/6] MEDIUM: proxy: Store server_id_hdr_name as a `struct ist`

The server_id_hdr_name is already processed as an ist in various locations lets
also just store it as such.

see 0643b0e7e ("MINOR: proxy: Make `header_unique_id` a `struct ist`") for a
very similar past commit.
---
 include/haproxy/proxy-t.h |  3 +--
 src/cfgparse-listen.c |  9 -
 src/mux_fcgi.c| 11 +--
 src/mux_h1.c  |  8 
 src/mux_h2.c  |  8 
 src/proxy.c   |  8 +++-
 6 files changed, 21 insertions(+), 26 deletions(-)

diff --git a/include/haproxy/proxy-t.h b/include/haproxy/proxy-t.h
index 805e1b452..80431757e 100644
--- a/include/haproxy/proxy-t.h
+++ b/include/haproxy/proxy-t.h
@@ -354,8 +354,7 @@ struct proxy {
struct net_addr except_xot_net; /* don't x-original-to for this 
address. */
struct ist fwdfor_hdr_name; /* header to use - 
default: "x-forwarded-for" */
struct ist orgto_hdr_name;  /* header to use - 
default: "x-original-to" */
-   char *server_id_hdr_name;   /* the header to use to 
send the server id (name) */
-   int server_id_hdr_len;  /* the length of the id 
(name) header... name */
+   struct ist server_id_hdr_name;   /* the header to use 
to send the server id (name) */
int conn_retries;   /* maximum number of connect 
retries */
unsigned int retry_type;/* Type of retry allowed */
int redispatch_after;   /* number of retries before 
redispatch */
diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c
index 121f1deac..216e6d8d5 100644
--- a/src/cfgparse-listen.c
+++ b/src/cfgparse-listen.c
@@ -1383,12 +1383,11 @@ int cfg_parse_listen(const char *file, int linenum, 
char **args, int kwm)
}
 
/* set the desired header name, in lower case */
-   free(curproxy->server_id_hdr_name);
-   curproxy->server_id_hdr_name = strdup(args[1]);
-   if (!curproxy->server_id_hdr_name)
+   istfree(>server_id_hdr_name);
+   curproxy->server_id_hdr_name = istdup(ist(args[1]));
+   if (!isttest(curproxy->server_id_hdr_name))
goto alloc_error;
-   curproxy->server_id_hdr_len  = 
strlen(curproxy->server_id_hdr_name);
-   ist2bin_lc(curproxy->server_id_hdr_name, 
ist2(curproxy->server_id_hdr_name, curproxy->server_id_hdr_len));
+   ist2bin_lc(istptr(curproxy->server_id_hdr_name), 
curproxy->server_id_hdr_name);
}
else if (strcmp(args[0], "block") == 0) {
ha_alert("parsing [%s:%d] : The '%s' directive is not supported 
anymore since HAProxy 2.1. Use 'http-request deny' which uses the exact same 
syntax.\n", file, linenum, args[0]);
diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c
index b5b280749..a22bc9391 100644
--- a/src/mux_fcgi.c
+++ b/src/mux_fcgi.c
@@ -2043,8 +2043,7 @@ static size_t fcgi_strm_send_params(struct fcgi_conn 
*fconn, struct fcgi_strm *f
}
 
/* Skip header if same name is used to 
add the server name */
-   if (fconn->proxy->server_id_hdr_name &&
-   isteq(p.n, 
ist2(fconn->proxy->server_id_hdr_name, fconn->proxy->server_id_hdr_len)))
+   if 
(isttest(fconn->proxy->server_id_hdr_name) && isteq(p.n, 
fconn->proxy->server_id_hdr_name))
break;
 
memcpy(trash.area, "http_", 5);
@@ -2062,15 +2061,15 @@ static size_t fcgi_strm_send_params(struct fcgi_conn 
*fconn, struct fcgi_strm *f
break;
 
case HTX_BLK_EOH:
-   if (fconn->proxy->server_id_hdr_name) {
+   if (isttest(fconn->proxy->server_id_hdr_name)) {
struct server *srv = 
objt_server(fconn->conn->target);
 
if (!srv)
    goto done;
 
-       memcpy(trash.area, "http_", 5);
-       memcpy(trash.area+5, 
fconn->proxy->server_id_hdr_name, fconn->proxy->server_id_hdr_len);
-   p.n = ist2(trash.area, 
fconn->proxy->server_id_hdr_len+5);
+   p.n = ist2(trash.area, 0);
+   istcat(, ist("http_"), trash.size);
+   istcat(, 
fconn->proxy->serv

[PATCH 3/6] MINOR: proxy: Store orgto_hdr_name as a `struct ist`

The orgto_hdr_name is already processed as an ist in `http_process_request`,
lets also just store it as such.

see 0643b0e7e ("MINOR: proxy: Make `header_unique_id` a `struct ist`") for a
very similar past commit.
---
 include/haproxy/proxy-t.h |  3 +--
 src/cfgparse-listen.c | 14 ++
 src/http_ana.c|  3 +--
 src/proxy.c   |  8 +++-
 4 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/include/haproxy/proxy-t.h b/include/haproxy/proxy-t.h
index 8277c098e..805e1b452 100644
--- a/include/haproxy/proxy-t.h
+++ b/include/haproxy/proxy-t.h
@@ -353,8 +353,7 @@ struct proxy {
struct net_addr except_xff_net; /* don't x-forward-for for this 
address. */
struct net_addr except_xot_net; /* don't x-original-to for this 
address. */
struct ist fwdfor_hdr_name; /* header to use - 
default: "x-forwarded-for" */
-   char *orgto_hdr_name;   /* header to use - default: 
"x-original-to" */
-   int orgto_hdr_len;  /* length of "x-original-to" 
header */
+   struct ist orgto_hdr_name;  /* header to use - 
default: "x-original-to" */
char *server_id_hdr_name;   /* the header to use to 
send the server id (name) */
int server_id_hdr_len;  /* the length of the id 
(name) header... name */
int conn_retries;   /* maximum number of connect 
retries */
diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c
index d858c3446..121f1deac 100644
--- a/src/cfgparse-listen.c
+++ b/src/cfgparse-listen.c
@@ -2399,11 +2399,10 @@ int cfg_parse_listen(const char *file, int linenum, 
char **args, int kwm)
 
curproxy->options |= PR_O_ORGTO;
 
-   free(curproxy->orgto_hdr_name);
-   curproxy->orgto_hdr_name = strdup(DEF_XORIGINALTO_HDR);
-   if (!curproxy->orgto_hdr_name)
+   istfree(>orgto_hdr_name);
+   curproxy->orgto_hdr_name = 
istdup(ist(DEF_XORIGINALTO_HDR));
+   if (!isttest(curproxy->orgto_hdr_name))
goto alloc_error;
-   curproxy->orgto_hdr_len  = strlen(DEF_XORIGINALTO_HDR);
curproxy->except_xot_net.family = AF_UNSPEC;
 
/* loop to go through arguments - start at 2, since 0+1 
= "option" "originalto" */
@@ -2441,11 +2440,10 @@ int cfg_parse_listen(const char *file, int linenum, 
char **args, int kwm)
err_code |= ERR_ALERT | 
ERR_FATAL;
goto out;
}
-   free(curproxy->orgto_hdr_name);
-   curproxy->orgto_hdr_name = 
strdup(args[cur_arg+1]);
-   if (!curproxy->orgto_hdr_name)
+   istfree(>orgto_hdr_name);
+   curproxy->orgto_hdr_name = 
istdup(ist(args[cur_arg+1]));
+   if (!isttest(curproxy->orgto_hdr_name))
goto alloc_error;
-   curproxy->orgto_hdr_len  = 
strlen(curproxy->orgto_hdr_name);
cur_arg += 2;
} else {
/* unknown suboption - catchall */
diff --git a/src/http_ana.c b/src/http_ana.c
index f02b8446b..83711482f 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -711,8 +711,7 @@ int http_process_request(struct stream *s, struct channel 
*req, int an_bit)
 */
if ((sess->fe->options | s->be->options) & PR_O_ORGTO) {
const struct sockaddr_storage *dst = si_dst(cs_si(s->csf));
-   struct ist hdr = ist2(s->be->orgto_hdr_len ? 
s->be->orgto_hdr_name : sess->fe->orgto_hdr_name,
- s->be->orgto_hdr_len ? 
s->be->orgto_hdr_len  : sess->fe->orgto_hdr_len);
+   struct ist hdr = isttest(s->be->orgto_hdr_name) ? 
s->be->orgto_hdr_name : sess->fe->orgto_hdr_name;
 
if (dst && dst->ss_family == AF_INET) {
/* Add an X-Original-To header unless the destination 
IP is
diff --git a/src/proxy.c b/src/proxy.c
index 53ca5db29..f051768ac 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -1436,7 +1436,7 @@ void proxy_free_defaults(struct proxy *defproxy)
ha_free(>defbe.name);
ha_free(>conn_src.iface_name);
istfree(>fwdfor_hdr_name);
-   ha_free(>orgto

[PATCH 2/6] MINOR: proxy: Store fwdfor_hdr_name as a `struct ist`