Re: [PATCH] JA3 TLS Fingerprinting (take 2)
Hello Willy, On Thu, Aug 26, 2021 at 8:40 PM Willy Tarreau wrote: > Hi Marcin, > > On Thu, Aug 26, 2021 at 06:56:20PM +0200, Marcin Deranek wrote: > > Hi Willy, > (...) > > No worries. Hopefully soon this will get merged. Attaching latest patches > > with all modification included. > > Thanks for detailing all the points. I trust that you did them as you > said, in the worst case it will just lead to bug fixes, that's fine. I > quickly glanced over all of it a last time and think it's OK, so I've > pushed it No fancy stuff just corrections as promised :-)) Thank you. I think I tested this fairly well, so I do not expect bugfixes.. but you never know :-) > I just wish I had pushed into a temporary branch first to see if the CI > complains on certain openssl versions, but fortunately it went fine :-) > Patches do not touch OpenSSL code, so if things worked before they should work after :-) BTW: You recall I "complained" about ssl_fc_cipherlist_xxh value being pre-calculated during TLS Client Hello message parsing which I found a bit "strange". One of the reasons could be that ssl_fc_cipherlist_str fallsback to that value (ssl_fc_cipherlist_xxh) when it cannot provide names due to OpenSSL limitations. > I changed the level of the last patch from MINOR to MEDIUM as it deprecates > a config setting and will result in emitting a new warning for some users, > thus it will have a visible impact. > I see. Thank you once again! Marcin Deranek
Re: [PATCH] JA3 TLS Fingerprinting (take 2)
Hi Willy, Thank you for the reply. On Tue, Aug 24, 2021 at 8:12 PM Willy Tarreau wrote: > Hi Marcin, > > I'm finally back to your patch set! Overall that looks fine, but I have > some comments, mostly cosmetic. > > > From b3a254b41411f22307a622250a6e95ac39fefee8 Mon Sep 17 00:00:00 2001 > > From: Marcin Deranek > > Date: Mon, 12 Jul 2021 14:16:55 +0200 > > Subject: [PATCH 1/5] MEDIUM: ssl: Capture more info from Client Hello > (...) > > > diff --git a/include/haproxy/ssl_sock-t.h b/include/haproxy/ssl_sock-t.h > > index 5acedcfc5..4b993894a 100644 > > --- a/include/haproxy/ssl_sock-t.h > > +++ b/include/haproxy/ssl_sock-t.h > > @@ -202,8 +202,16 @@ struct ssl_sock_msg_callback { > > /* This memory pool is used for capturing clienthello parameters. */ > > struct ssl_capture { > > unsigned long long int xxh64; > > - unsigned char ciphersuite_len; > > - char ciphersuite[VAR_ARRAY]; > > + unsigned short int protocol_version; > > + unsigned short int ciphersuite_len; > > + unsigned short int extensions_len; > > + unsigned short int ec_len; > > No need to mention "int" after "short" or "long", that's implicit. I > guess you did it because of the "unsigned long long int" above but that > is not needed. Since 2.4 we have shorter type names like "ushort", "uint", > "uchar" etc that we're progressively trying to standardize the new code > to, but they haven't slipped into older code yet. This could make sense > here since the whole structure is being replaced. But that's not an > obligation :-) > Converted all to new types, so we are future proof :-) > > diff --git a/src/ssl_sock.c b/src/ssl_sock.c > > index ba61243ac..2965a1446 100644 > > --- a/src/ssl_sock.c > > +++ b/src/ssl_sock.c > > @@ -1652,7 +1652,16 @@ static void ssl_sock_parse_clienthello(struct > connection *conn, int write_p, int > > struct ssl_capture *capture; > > unsigned char *msg; > > unsigned char *end; > > - size_t rec_len; > > + unsigned char *extensions_end; > > + unsigned char *ec_start = NULL; > > + unsigned char *ec_formats_start = NULL; > > + unsigned char *list_end; > > + unsigned short int protocol_version; > > + unsigned short int extension_id; > > + unsigned short int ec_len = 0; > > Same here for the type names. > Also converted. > > @@ -1747,10 +1763,106 @@ static void ssl_sock_parse_clienthello(struct > connection *conn, int write_p, int > > capture->xxh64 = XXH64(msg, rec_len, 0); > > > > /* Capture the ciphersuite. */ > > - capture->ciphersuite_len = (global_ssl.capture_cipherlist < > rec_len) ? > > - global_ssl.capture_cipherlist : rec_len; > > - memcpy(capture->ciphersuite, msg, capture->ciphersuite_len); > > + capture->ciphersuite_len = MIN(global_ssl.capture_cipherlist, > rec_len); > > + capture->ciphersuite_offset = 0; > > + memcpy(capture->data, msg, capture->ciphersuite_len); > > + msg += rec_len; > > + offset += capture->ciphersuite_len; > > + > > + /* Initialize other data */ > > + capture->protocol_version = protocol_version; > > + capture->extensions_len = 0; > > + capture->extensions_offset = 0; > > + capture->ec_len = 0; > > + capture->ec_offset = 0; > > + capture->ec_formats_len = 0; > > + capture->ec_formats_offset = 0; > > Given the number of fields to preset to zero, you should instead replace > the previous pool_alloc() call with a pool_zalloc(), it would be more > future-proof and safer against the risk of accidentally missing one > such field when more are added. > Understood. replaced pool_alloc() with pool_zalloc() are removed initialization of individual fields as they are not needed anymore. > > + /* Expect 2 bytes extension id + 2 bytes extension size */ > > + msg += 2 + 2; > > + if (msg + rec_len > extensions_end || msg + rec_len < msg) > > + goto store_capture; > > + if (extension_id == 0x000a) { > > + /* Elliptic Curves */ > > Could you put a link here to the registry where you found this definition ? > I guess it comes from an RFC or anything, but it does help when some > extensions are needed or when dealing with interoperability issues, to > figure if any other values need to be considered. > Added l
Re: [PATCH] JA3 TLS Fingerprinting (take 2)
Hi Willy, No worries.. See an attached the newest set of patches rebased against master (some conflicts appeared in the meantime). Regards, Marcin Deranek On Tue, Aug 17, 2021 at 9:58 AM Willy Tarreau wrote: > Hi Marcin, > > On Mon, Aug 16, 2021 at 01:55:02PM +0200, Marcin Deranek wrote: > > Hi, > > > > Do you have any update on merging this? > > Sorry, I think we've missed it :-( Worse, I was wondering if you > managed to make any progress on it :-/ > > I'm currently working on preparing a set of stable branches, I'll have > a look after that. > > Thanks for the ping! > Willy > From 0a26066b685d52dae818d2e369aa48c3119c4769 Mon Sep 17 00:00:00 2001 From: Marcin Deranek Date: Tue, 13 Jul 2021 14:08:56 +0200 Subject: [PATCH 4/5] MINOR: sample: Add be2hex converter Add be2hex converter to convert big-endian binary data into hex string with optional string separators. --- doc/configuration.txt | 14 reg-tests/converter/be2hex.vtc | 60 ++ src/sample.c | 54 ++ 3 files changed, 128 insertions(+) create mode 100644 reg-tests/converter/be2hex.vtc diff --git a/doc/configuration.txt b/doc/configuration.txt index babd33a12..f3e575988 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -16104,6 +16104,20 @@ be2dec(,,[]) bin(01020304050607),be2dec(-,2,1) # 258-772-1286 bin(01020304050607),be2dec(,2,1) # 2587721286 +be2hex([],[],[]) + Converts big-endian binary input sample to a hex string containing two hex + digits per input byte. It is used to log or transfer hex dumps of some + binary input data in a way that can be reliably transferred (e.g. an SSL ID + can be copied in a header). is put every binary + input bytes if specified. flag indicates whatever binary input is + truncated at boundaries. + + Example: + bin(01020304050607),be2hex # 01020304050607 + bin(01020304050607),be2hex(:,2)# 0102:0304:0506:07 + bin(01020304050607),be2hex(--,2,1) # 0102--0304--0506 + bin(0102030405060708),be2hex(,3,1) # 010203040506 + bool Returns a boolean TRUE if the input value of type signed integer is non-null, otherwise returns FALSE. Used in conjunction with and(), it can be diff --git a/reg-tests/converter/be2hex.vtc b/reg-tests/converter/be2hex.vtc new file mode 100644 index 0..86e504292 --- /dev/null +++ b/reg-tests/converter/be2hex.vtc @@ -0,0 +1,60 @@ +varnishtest "be2hex converter Test" + +feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'" +feature ignore_unknown_macro + +server s1 { + rxreq + txresp +} -repeat 3 -start + +haproxy h1 -conf { +defaults + mode http + timeout connect 1s + timeout client 1s + timeout server 1s + +frontend fe + bind "fd@${fe}" + + requests + http-request set-var(txn.input) req.hdr(input) + + http-response set-header be2hex "%[var(txn.input),be2hex,lower]" + http-response set-header be2hex-1 "%[var(txn.input),be2hex(:,1),lower]" + http-response set-header be2hex-2 "%[var(txn.input),be2hex(--,3),lower]" + http-response set-header be2hex-3 "%[var(txn.input),be2hex(.,3,1),lower]" + + default_backend be + +backend be + server s1 ${s1_addr}:${s1_port} +} -start + +client c1 -connect ${h1_fe_sock} { + txreq -url "/" \ + -hdr "input:" + rxresp + expect resp.status == 200 + expect resp.http.be2hex == "" + expect resp.http.be2hex-1 == "" + expect resp.http.be2hex-2 == "" + expect resp.http.be2hex-3 == "" + txreq -url "/" \ + -hdr "input: 0123456789" + rxresp + expect resp.status == 200 + expect resp.http.be2hex == "30313233343536373839" + expect resp.http.be2hex-1 == "30:31:32:33:34:35:36:37:38:39" + expect resp.http.be2hex-2 == "303132--333435--363738--39" + expect resp.http.be2hex-3 == "303132.333435.363738" + txreq -url "/" \ + -hdr "input: abcdefghijklmnopqrstuvwxyz" + rxresp + expect resp.status == 200 + expect resp.http.be2hex == "6162636465666768696a6b6c6d6e6f707172737475767778797a" + expect resp.http.be2hex-1 == "61:62:63:64:65:66:67:68:69:6a:6b:6c:6d:6e:6f:70:71:72:73:74:75:76:77:78:79:7a" + expect resp.http.be2hex-2 == "616263--646566--676869--6a6b6c--6d6e6f--707172--737475--767778--797a" + expect resp.http.be2hex-3 == "616263.646566.676869.6a6b6c.6d6e6f.707172.737475.767778" +} -run diff --git a/src/sample.c b/src/sample.c index 5b7ad8b34..00f296b6c 100644 --- a/src/sample.c +++ b/src/sample.c @@ -2113,6 +2113,59 @@ static int sample_conv_be2dec(const struct arg *args, struct sample *smp, void * return 1; } +static int sample_conv_be2hex_check(struct arg *args, struct sample_conv *conv, +
Re: [External] Re: [PATCH] JA3 TLS Fingerprinting (take 2)
Hi, Do you have any update on merging this? Regards, Marcin Deranek On Thu, Jul 15, 2021 at 2:28 PM Marcin Deranek wrote: > Hi Tim, > > Updated (see attachments). Other patches did not change. > Regards, > > Marcin Deranek > > On Thu, Jul 15, 2021 at 10:20 AM Tim Düsterhus wrote: > >> Marcin, >> >> On 7/14/21 2:01 PM, Marcin Deranek wrote: >> > Thank you for all comments I have received regarding JA3 Fingerprinting >> > patches. Here is the new set of patches which incorporated all your >> > suggestions. >> >> Sorry I gave a little outdated advice regarding the reg-tests. For any >> new tests please use: >> >>feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'" >> >> instead of >> >>#REQUIRE_VERSION=2.5 >> >> Other than that the tests LGTM from a glance. I didn't look at your C >> and I also didn't (yet) compare the tests against the documentation you >> have written. >> >> Best regards >> Tim Düsterhus >> > > > -- > Marcin Deranek > Senior Site Reliability Engineer > [image: Booking.com] <https://www.booking.com/> > Making it easier for everyone > to experience the world. >
Re: [External] Re: [PATCH] JA3 TLS Fingerprinting (take 2)
Hi Tim, Updated (see attachments). Other patches did not change. Regards, Marcin Deranek On Thu, Jul 15, 2021 at 10:20 AM Tim Düsterhus wrote: > Marcin, > > On 7/14/21 2:01 PM, Marcin Deranek wrote: > > Thank you for all comments I have received regarding JA3 Fingerprinting > > patches. Here is the new set of patches which incorporated all your > > suggestions. > > Sorry I gave a little outdated advice regarding the reg-tests. For any > new tests please use: > >feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'" > > instead of > >#REQUIRE_VERSION=2.5 > > Other than that the tests LGTM from a glance. I didn't look at your C > and I also didn't (yet) compare the tests against the documentation you > have written. > > Best regards > Tim Düsterhus > -- Marcin Deranek Senior Site Reliability Engineer [image: Booking.com] <https://www.booking.com/> Making it easier for everyone to experience the world. From 87dcccaa9d3fbd4474b256bf6623323621c3144b Mon Sep 17 00:00:00 2001 From: Marcin Deranek Date: Tue, 13 Jul 2021 14:05:24 +0200 Subject: [PATCH 3/5] MINOR: sample: Add be2dec converter Add be2dec converter which allows to build JA3 compatible TLS fingerprints by converting big-endian binary data into string separated unsigned integers eg. http-request set-header X-SSL-JA3 %[ssl_fc_protocol_hello_id],\ %[ssl_fc_cipherlist_bin(1),be2dec(-,2)],\ %[ssl_fc_extlist_bin(1),be2dec(-,2)],\ %[ssl_fc_eclist_bin(1),be2dec(-,2)],\ %[ssl_fc_ecformats_bin,be2dec(-,1)] --- doc/configuration.txt | 12 +++ reg-tests/converter/be2dec.vtc | 56 + src/sample.c | 57 ++ 3 files changed, 125 insertions(+) create mode 100644 reg-tests/converter/be2dec.vtc diff --git a/doc/configuration.txt b/doc/configuration.txt index ecbbcdd04..d39e90752 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -16064,6 +16064,18 @@ base64 an SSL ID can be copied in a header). For base64url("URL and Filename Safe Alphabet" (RFC 4648)) variant see "ub64enc". +be2dec(,,[]) + Converts a binary input sample to a string containing an unsigned integer + number per input bytes. is put every + binary input bytes if specified. flag indicates whatever binary + input is truncated at boundaries. maximum value is + limited by the size of long long int (8 bytes). + + Example: + bin(01020304050607),be2dec(:,2) # 258:772:1286:7 + bin(01020304050607),be2dec(-,2,1) # 258-772-1286 + bin(01020304050607),be2dec(,2,1) # 2587721286 + bool Returns a boolean TRUE if the input value of type signed integer is non-null, otherwise returns FALSE. Used in conjunction with and(), it can be diff --git a/reg-tests/converter/be2dec.vtc b/reg-tests/converter/be2dec.vtc new file mode 100644 index 0..bdb903523 --- /dev/null +++ b/reg-tests/converter/be2dec.vtc @@ -0,0 +1,56 @@ +varnishtest "be2dec converter Test" + +feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'" +feature ignore_unknown_macro + +server s1 { + rxreq + txresp +} -repeat 3 -start + +haproxy h1 -conf { +defaults + mode http + timeout connect 1s + timeout client 1s + timeout server 1s + +frontend fe + bind "fd@${fe}" + + requests + http-request set-var(txn.input) req.hdr(input) + + http-response set-header be2dec-1 "%[var(txn.input),be2dec(:,1)]" + http-response set-header be2dec-2 "%[var(txn.input),be2dec(-,3)]" + http-response set-header be2dec-3 "%[var(txn.input),be2dec(::,3,1)]" + + default_backend be + +backend be + server s1 ${s1_addr}:${s1_port} +} -start + +client c1 -connect ${h1_fe_sock} { + txreq -url "/" \ + -hdr "input:" + rxresp + expect resp.status == 200 + expect resp.http.be2dec-1 == "" + expect resp.http.be2dec-2 == "" + expect resp.http.be2dec-3 == "" + txreq -url "/" \ + -hdr "input: 0123456789" + rxresp + expect resp.status == 200 + expect resp.http.be2dec-1 == "48:49:50:51:52:53:54:55:56:57" + expect resp.http.be2dec-2 == "3158322-3355701-3553080-57" + expect resp.http.be2dec-3 == "3158322::3355701::3553080" + txreq -url "/" \ + -hdr "input: abcdefghijklmnopqrstuvwxyz" + rxresp + expect resp.status == 200 + expect resp.http.be2dec-1 == "97:98:99:100:101:102:103:104:105:106:107:108:109:110:111:112:113:114:115:116:117:118:119:120:121:122" + expect resp.http.be2dec-2 == "6382179-6579558-6776937-6974316-7171695-7369074-7566453-7763832-31098" + expect resp.http.be2dec-3 == "6382179::6579558::6776937::6974316::7171695::7369074::7566453::7763832" +} -run diff --git a/src/sample.c b/src/sample.c index d02034cf0..
[PATCH] JA3 TLS Fingerprinting (take 2)
Hi, Thank you for all comments I have received regarding JA3 Fingerprinting patches. Here is the new set of patches which incorporated all your suggestions. Willy: I lowered memory requirements for ssl_capture (now 40 extra bytes), but I did not go with the lowest as you suggested (unsigned char for length/unsigned short for offset). Potentially this would work just fine, yet specification allows to exceed that ( https://mta.openssl.org/pipermail/openssl-dev/2015-September/002860.html) and personally I'm more in favour of sticking to standards as things could bite us in the future. Dropping precalculated xxh64 hash would allow us to go as low as 28 bytes if we care a lot about memory. Regards, Marcin Deranek From 14a84a136e7e52957ae44fecaec432bdb9e3f4c9 Mon Sep 17 00:00:00 2001 From: Marcin Deranek Date: Tue, 13 Jul 2021 14:05:24 +0200 Subject: [PATCH 3/5] MINOR: sample: Add be2dec converter Add be2dec converter which allows to build JA3 compatible TLS fingerprints by converting big-endian binary data into string separated unsigned integers eg. http-request set-header X-SSL-JA3 %[ssl_fc_protocol_hello_id],\ %[ssl_fc_cipherlist_bin(1),be2dec(-,2)],\ %[ssl_fc_extlist_bin(1),be2dec(-,2)],\ %[ssl_fc_eclist_bin(1),be2dec(-,2)],\ %[ssl_fc_ecformats_bin,be2dec(-,1)] --- doc/configuration.txt | 12 +++ reg-tests/converter/be2dec.vtc | 50 + src/sample.c | 57 ++ 3 files changed, 119 insertions(+) create mode 100644 reg-tests/converter/be2dec.vtc diff --git a/doc/configuration.txt b/doc/configuration.txt index ecbbcdd04..d39e90752 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -16064,6 +16064,18 @@ base64 an SSL ID can be copied in a header). For base64url("URL and Filename Safe Alphabet" (RFC 4648)) variant see "ub64enc". +be2dec(,,[]) + Converts a binary input sample to a string containing an unsigned integer + number per input bytes. is put every + binary input bytes if specified. flag indicates whatever binary + input is truncated at boundaries. maximum value is + limited by the size of long long int (8 bytes). + + Example: + bin(01020304050607),be2dec(:,2) # 258:772:1286:7 + bin(01020304050607),be2dec(-,2,1) # 258-772-1286 + bin(01020304050607),be2dec(,2,1) # 2587721286 + bool Returns a boolean TRUE if the input value of type signed integer is non-null, otherwise returns FALSE. Used in conjunction with and(), it can be diff --git a/reg-tests/converter/be2dec.vtc b/reg-tests/converter/be2dec.vtc new file mode 100644 index 0..d329e01b3 --- /dev/null +++ b/reg-tests/converter/be2dec.vtc @@ -0,0 +1,50 @@ +varnishtest "be2dec converter Test" + +#REQUIRE_VERSION=2.5 + +feature ignore_unknown_macro + +server s1 { + rxreq + txresp +} -repeat 2 -start + +haproxy h1 -conf { +defaults + mode http + timeout connect 1s + timeout client 1s + timeout server 1s + +frontend fe + bind "fd@${fe}" + + requests + http-request set-var(txn.input) req.hdr(input) + + http-response set-header be2dec-1 "%[var(txn.input),be2dec(:,1)]" + http-response set-header be2dec-2 "%[var(txn.input),be2dec(-,3)]" + http-response set-header be2dec-3 "%[var(txn.input),be2dec(::,3,1)]" + + default_backend be + +backend be + server s1 ${s1_addr}:${s1_port} +} -start + +client c1 -connect ${h1_fe_sock} { + txreq -url "/" \ + -hdr "input: 0123456789" + rxresp + expect resp.status == 200 + expect resp.http.be2dec-1 == "48:49:50:51:52:53:54:55:56:57" + expect resp.http.be2dec-2 == "3158322-3355701-3553080-57" + expect resp.http.be2dec-3 == "3158322::3355701::3553080" + txreq -url "/" \ + -hdr "input: abcdefghijklmnopqrstuvwxyz" + rxresp + expect resp.status == 200 + expect resp.http.be2dec-1 == "97:98:99:100:101:102:103:104:105:106:107:108:109:110:111:112:113:114:115:116:117:118:119:120:121:122" + expect resp.http.be2dec-2 == "6382179-6579558-6776937-6974316-7171695-7369074-7566453-7763832-31098" + expect resp.http.be2dec-3 == "6382179::6579558::6776937::6974316::7171695::7369074::7566453::7763832" +} -run diff --git a/src/sample.c b/src/sample.c index d02034cf0..5b7ad8b34 100644 --- a/src/sample.c +++ b/src/sample.c @@ -2057,6 +2057,62 @@ static int sample_conv_crypto_hmac(const struct arg *args, struct sample *smp, v #endif /* USE_OPENSSL */ +static int sample_conv_be2dec_check(struct arg *args, struct sample_conv *conv, +const char *file, int line, char **err) +{ + if (args[1].data.sint <= 0 || args[1].data.sint > sizeof(unsigned long long)) { + memprintf(err, "chunk_size out of [1..%ld] range (%lld)", sizeof(unsigned long long), args[1].data.sint); + return 0; + } + + if (args[2].data.sint != 0 &&
Re: [PATCH] JA3 TLS Fingerprinting
Hi Tim, Will try to include reg-tests for converters (first need to read on how to do it). I'm affraid fetchers are not really suitable for regtest. Jump label was already indented by 1 space. Thank you for feedback Tim. Regards, Marcin Deranek On Mon, Jul 12, 2021 at 5:21 PM Tim Düsterhus wrote: > Marcin, > > On 7/12/21 4:59 PM, Marcin Deranek wrote: > > Over a past few weeks I have been working on implementing JA3 compatible > > TLS Fingerprinting[1] in the HAProxy. You can find the outcome in > > attachments. Feel free to review/comment them. > > I can't comment on the correctness of the patches, but please add > reg-tests where possible. At the very least the new / updated converters > should should (must?) get a reg-test to ensure correctness. > > Also one minor remark regarding the first patch: Please indent jump > labels (store_capture:) by at least one space. This improves the > detection of diff hunk headers in some cases. > > Best regards > Tim Düsterhus
Re: [PATCH] JA3 TLS Fingerprinting
Hi Илья, Well, JA3 is one the approaches which seems to be a "standard" our there. If you want to build your own you are still able to do so if you wish, however using a "standard" gives you a benefit of interoperability between different software (eg. JA3 on Nginx is the same JA3 on HAProxy) and ability to exchange information in a unified manner (eg. somebody posts JA3 fingerprint of some malware which you can block right away without a need of discovering it). Regards, Marcin Deranek On Mon, Jul 12, 2021 at 7:37 PM Илья Шипицин wrote: > JA3 is good approach, but it lacks few ideas. > > we fingerprinted clients by "ssl ciphers" (all ciphers sent by client in > Client Helo) + "all client curves" (also sent by client). > > however you approach is flexible enough to be extended. > > пн, 12 июл. 2021 г. в 20:03, Marcin Deranek : > >> Hi, >> >> Over a past few weeks I have been working on implementing JA3 compatible >> TLS Fingerprinting[1] in the HAProxy. You can find the outcome in >> attachments. Feel free to review/comment them. >> Here are some choices I made which you should be aware of: >> - I decided to go with a "modular" approach where you can build JA3 >> compatible fingerprint with available fetchers/converters rather than a >> single JA3 fetcher. This makes approach more "reusable" in some other >> scenarios. >> - Each Client Hello related fetcher has option to include/exclude GREASE >> (RFC8701) values from the output. This is mainly for backward compatibility >> and ability to get "pure" data. I suspect in most cases people do not want >> GREASE values to be present in the output (which is not the case right now >> for cipherlist fetchers). >> - exclude_grease function allocates trash on demand depending on GREASE >> (RFC8701) values position in the list. We can get away without creating >> trash buffer if GREASE values are present at the very beginning and/or the >> very end of the list. I decided to allocate trash buffer only when it's >> really needed, so that's why it's creation is "hidden" inside exlude_grease >> function. >> - Now ssl_capture (next to ciphersuite) contains data about extensions, >> ec ciphers etc. One of the reasons I decided to merge all those values in a >> single ssl_capture buffer is easier control of buffer size limit. I think >> it's beneficial to have a single buffer limit for all those values rather >> than separate values for each. Having said that probably >> tune.ssl.capture-cipherlist-size needs to change it's name to eg. >> tune.ssl.capture-buffer-limit to better reflect it's function. >> - Instead of creating a new converter I decided to extend existing hex >> conveter to provide a similar functionality to bin2int. I thought this >> makes more sense as extended hex converter is fully backward compatible. It >> has to be noted that extended hex converter is not strictly necessary to >> produce JA3 TLS Fingerprint, but but might useful in some other scenarios. >> >> Example usage: >> http-request set-header X-SSL-JA3 >> %[ssl_fc_protocol_hello_id],%[ssl_fc_cipherlist_bin(1),bin2int(-,2)],%[ssl_fc_extlist_bin(1),bin2int(-,2)],%[ssl_fc_eclist_bin(1),bin2int(-,2)],%[ssl_fc_ecformats_bin,bin2int(-,1)] >> http-request set-header X-SSL-JA3-Hash >> %[req.fhdr(x-ssl-ja3),digest(md5),hex] >> >> Question: I noticed that during Client Hello parsing we calculate xxh64 >> right away and store it. Isn't better to calculate it when it's actually >> used? >> Regards, >> >> Marcin Deranek >> >> [1] https://github.com/salesforce/ja3 >> <https://urldefense.com/v3/__https://github.com/salesforce/ja3__;!!FzMMvhmfRQ!_kUdt3LbsvygC0QjXBrvhTfZfa8a50VMpTB_iWOVP3m7HA7HD9Jy41snhTOKb2EYjKpf$> >> >> -- Marcin Deranek Senior Site Reliability Engineer [image: Booking.com] <https://www.booking.com/> Making it easier for everyone to experience the world. -- Marcin Deranek Senior Site Reliability Engineer [image: Booking.com] <https://www.booking.com/> Making it easier for everyone to experience the world.
Re: [PATCH] JA3 TLS Fingerprinting
er complete these with other less useful variants like octal > or raw ints (e.g. to extract dates). Just like we could imagine supporting > some flavors of varints on input later if needed for some protocols. > Will add new converters and name them be2hex / be2dec (hex will stay intact). > > Example usage: > > http-request set-header X-SSL-JA3 > > > %[ssl_fc_protocol_hello_id],%[ssl_fc_cipherlist_bin(1),bin2int(-,2)],%[ssl_fc_extlist_bin(1),bin2int(-,2)],%[ssl_fc_eclist_bin(1),bin2int(-,2)],%[ssl_fc_ecformats_bin,bin2int(-,1)] > > http-request set-header X-SSL-JA3-Hash > > %[req.fhdr(x-ssl-ja3),digest(md5),hex] > > I think in the doc you should add an example showing how to match a > signature against those listed in file "lists/osx-nix-ja3.csv" in the > project. It will help you verify if the solution completely works and > is practically usable. Maybe it can involve intermediary variables > for example. > Thought about it although due to complexity decided not to include it. Potentially I could "spread it" over multiple lines to make it more readable. Regards, Marcin Deranek
[PATCH] JA3 TLS Fingerprinting
Hi, Over a past few weeks I have been working on implementing JA3 compatible TLS Fingerprinting[1] in the HAProxy. You can find the outcome in attachments. Feel free to review/comment them. Here are some choices I made which you should be aware of: - I decided to go with a "modular" approach where you can build JA3 compatible fingerprint with available fetchers/converters rather than a single JA3 fetcher. This makes approach more "reusable" in some other scenarios. - Each Client Hello related fetcher has option to include/exclude GREASE (RFC8701) values from the output. This is mainly for backward compatibility and ability to get "pure" data. I suspect in most cases people do not want GREASE values to be present in the output (which is not the case right now for cipherlist fetchers). - exclude_grease function allocates trash on demand depending on GREASE (RFC8701) values position in the list. We can get away without creating trash buffer if GREASE values are present at the very beginning and/or the very end of the list. I decided to allocate trash buffer only when it's really needed, so that's why it's creation is "hidden" inside exlude_grease function. - Now ssl_capture (next to ciphersuite) contains data about extensions, ec ciphers etc. One of the reasons I decided to merge all those values in a single ssl_capture buffer is easier control of buffer size limit. I think it's beneficial to have a single buffer limit for all those values rather than separate values for each. Having said that probably tune.ssl.capture-cipherlist-size needs to change it's name to eg. tune.ssl.capture-buffer-limit to better reflect it's function. - Instead of creating a new converter I decided to extend existing hex conveter to provide a similar functionality to bin2int. I thought this makes more sense as extended hex converter is fully backward compatible. It has to be noted that extended hex converter is not strictly necessary to produce JA3 TLS Fingerprint, but but might useful in some other scenarios. Example usage: http-request set-header X-SSL-JA3 %[ssl_fc_protocol_hello_id],%[ssl_fc_cipherlist_bin(1),bin2int(-,2)],%[ssl_fc_extlist_bin(1),bin2int(-,2)],%[ssl_fc_eclist_bin(1),bin2int(-,2)],%[ssl_fc_ecformats_bin,bin2int(-,1)] http-request set-header X-SSL-JA3-Hash %[req.fhdr(x-ssl-ja3),digest(md5),hex] Question: I noticed that during Client Hello parsing we calculate xxh64 right away and store it. Isn't better to calculate it when it's actually used? Regards, Marcin Deranek [1] https://github.com/salesforce/ja3 From 649105084c7019bc48ff508376c8da137bb1f53b Mon Sep 17 00:00:00 2001 From: Marcin Deranek Date: Mon, 12 Jul 2021 15:31:35 +0200 Subject: [PATCH 4/4] MEDIUM: sample: Extend functionality of hex converter Allow hex converter to separate output with string separator and group bytes, so more binary to hex transformations are possible. This gives hex converter the very same functionality bin2int converter has. Change is backward compatible. --- doc/configuration.txt | 12 +-- src/sample.c | 49 --- 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/doc/configuration.txt b/doc/configuration.txt index c4a187722..cd96bc6e8 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -16268,11 +16268,19 @@ fix_tag_value() tcp-request content set-var(txn.foo) req.payload(0,0),fix_tag_value(35) tcp-request content set-var(txn.bar) req.payload(0,0),fix_tag_value(MsgType) -hex +hex([],[],[]) Converts a binary input sample to a hex string containing two hex digits per input byte. It is used to log or transfer hex dumps of some binary input data in a way that can be reliably transferred (e.g. an SSL ID can be copied in a - header). + header). is put every binary input bytes if + specified. flag indicates whatever binary input is truncated at + boundaries. + + Example: + bin(01020304050607),hex # 01020304050607 + bin(01020304050607),hex(:,2)# 0102:0304:0506:07 + bin(01020304050607),hex(--,2,1) # 0102--0304--0506 + bin(0102030405060708),hex(,3,1) # 010203040506 hex2i Converts a hex string containing two hex digits per input byte to an diff --git a/src/sample.c b/src/sample.c index a34aee51a..dbd62c5a9 100644 --- a/src/sample.c +++ b/src/sample.c @@ -2113,18 +2113,53 @@ static int sample_conv_bin2int(const struct arg *args, struct sample *smp, void return 1; } -static int sample_conv_bin2hex(const struct arg *arg_p, struct sample *smp, void *private) +static int sample_conv_hex_check(struct arg *args, struct sample_conv *conv, + const char *file, int line, char **err) +{ + if (args[1].data.sint <= 0 && (args[0].data.str.data > 0 || args[2].data.sint != 0)) { + memprintf(err, "chunk_size needs to be positive (%lld)", args[1].d
Re: [External] Re: [RFC PATCH] Improved moving averages
Great, thanks WIlly. Enjoy! :-) Marcin Deranek On Sat, May 16, 2020 at 10:40 PM Willy Tarreau wrote: > Hi Marcin, > > On Sat, May 16, 2020 at 04:31:30PM +0200, Marcin Deranek wrote: > > Actually I split this into 3 (see attachments): > > 1. Adds improved swrate_rate_dynamic function > > 2. Adds proper req_tot metric for a server (as we have for > frontend/backend) > > 3. Updates the code to use the new function for existing *time metrics. > > They should behave the same except they should be much more accurate at > the > > very beginning. This one depends on 1 & 2. > > Regards, > > Nothing else to say but "looks perfect, now merged" :-) > > Thanks! > Willy > -- Marcin Deranek Senior System Administrator Booking.com B.V. Singel 540 Amsterdam 1017AZ Netherlands Direct +31207243362 [image: Booking.com] <https://www.booking.com/> Making it easier for everyone to experience the world since 1996 43 languages, 214+ offices worldwide, 141,000+ global destinations, 29 million reported listings Subsidiary of Booking Holdings Inc. (NASDAQ: BKNG)
Re: [External] Re: [RFC PATCH] Improved moving averages
Hi Willy, On Fri, May 15, 2020 at 2:45 PM Willy Tarreau wrote: > What I'd like to see for the ctime etc is: > - average over the number of samples like you did, but still per request > ; > - possibly new sets of per-connection metrics if these have some values > to you (I guess they do). > I think these which we have are sufficient: for HTTP mode we measure on a per-request basis and for TCP mode we measure on connection basis (I hope cum_lbconn is the right counter for TCP). > What will happen with two sets of metrics is that both will be exactly > identical in TCP. But that's precisely because HTTP makes a difference that > we want to split them! > I don't want to change this, so I kept existing metrics (ctime, ttime etc.), but as I mentioned above they are calculated a bit differently for HTTP and TCP modes. > > I don't mind creating additional metrics if they make sense of course. > One > > of them (which could help to avoid exceptions) would be req_tot for a > > server (it's available for backend). > > Wow, I thought we already had it. When I say that we really need to invest > time digging into the existing metrics, I'm not kidding. Yes sure, feel > free > to add this one as well. > It feels like a hack by summing up all hrsp_* metrics. Patch 0002 addresses that. > > To dynamically scale n, req_tot is > > much more suitable than lbconn for http mode (lbconn is incremented much > > earlier than req_tot creating a time window when metrics can be > incorrectly > > calculated leading to wrong averages). > > It's not the worst issue, the worst issue is that lbconn is only > incremented > on load balanced connections; as soo as you have cookie-based stickiness or > use-server rules, lbconn doesn't move. > > I've just looked on the stats page and I'm seeing a "Cum HTTP responses" > field for the servers in the tooltip, which is accurately incremented. I > just don't know what counter it is, I would have thought it's the request > counter. > See above. > > Of course if you still think it's a > > good idea to separate them (personally I'm not convinced about this > unless > > you don't want to change behaviour of existing metric etc.) I can make > new > > metrics eg. ctime_avg. > > Just to be sure we're talking about the same thing, I'm convinced about the > benefit of splitting such averages per-request and per-connection, and of > changing them to use your sliding average. > I kept things as-is, so HTTP show *time metric per-request and TCP per connection. > I'm pretty sure I tried the two and found that rounding up was better. > I think it's related to the loss of precision caused by exponentiating > the old value, which would disappear after the second sample if only > rounded. But I could be wrong, that's a bit old now. I'd suggest to > proceed as you find the most accurate between the two. > Left rounding up. > > > I'd also ask you to split your patch in 2 : > > > > > > - one which improves the freq_ctr with the new function(s) > > > - one which adds the new metric, its update and reporting to the > > > various call places. > > > > > > In addition, I think there will be less changes with a separate metric. > > > > > > > Will do. Thank you for the review. > > You're welcome, thanks for this! > Actually I split this into 3 (see attachments): 1. Adds improved swrate_rate_dynamic function 2. Adds proper req_tot metric for a server (as we have for frontend/backend) 3. Updates the code to use the new function for existing *time metrics. They should behave the same except they should be much more accurate at the very beginning. This one depends on 1 & 2. Regards, Marcin Deranek Senior System Administrator Booking.com B.V. Singel 540 Amsterdam 1017AZ Netherlands Direct +31207243362 [image: Booking.com] <https://www.booking.com/> Making it easier for everyone to experience the world since 1996 43 languages, 214+ offices worldwide, 141,000+ global destinations, 29 million reported listings Subsidiary of Booking Holdings Inc. (NASDAQ: BKNG) From af5e10eeecd9c146861ff7bd8bcaec041eb77d5d Mon Sep 17 00:00:00 2001 From: Marcin Deranek Date: Fri, 15 May 2020 18:26:18 +0200 Subject: [PATCH 1/3] MINOR stats: Prepare for more accurate moving averages Add swrate_add_dynamic function which is similar to swrate_add, but more accurate when calculating moving averages when not enough samples have been processed yet. --- include/proto/freq_ctr.h | 18 ++ 1 file changed, 18 insertions(+) diff --git a/include/proto/freq_ctr.h b/include/proto/freq_ctr.h
Re: [External] Re: [RFC PATCH] Improved moving averages
Hi Willy, On Wed, May 13, 2020 at 11:25 AM Willy Tarreau wrote: > Hi Marcin! > > On Tue, May 12, 2020 at 12:17:19PM +0200, Marcin Deranek wrote: > > Hi, > > > > Not a long ago while investigating some issues with one of the services I > > stumbled across Connect Time (based on ctime metric) graphs (see > > ctime-problem.png). It turned out that metrics were nowhere near reality > - > > they were trying to reach real average value, but never got there as each > > reload reset it back to 0. Keep in mind this was happening on a > > multi-process HAProxy setup for a service with relatively low/medium > amount > > of traffic. I did a bit of investigation, tested different scenarios / > > algorithms, selected the most optimal one and deployed it on one of our > > load balancers (see low-traffic-compare.png and > > medium-traffic-compare.png). ctime represents current algorithm and > ctime2 > > represents new algorithm for calculating moving averages. As you can see, > > differences can be dramatic over long periods of time. > > The improvements are indeed really nice! > Glad to hear that! > > Drops of ctime > > metric represent reloads of an HAProxy instance. Spikes of ctime2 are due > > to http-reuse option - after reloading a new instance cannot reuse > existing > > connections, so it has to establish new connections, so timing goes up > > (this is expected). > > See a proposed patch. Few comments about the patch: > > - The patch changes behaviour of moving average metrics generation (eg. > > ctime, ttime) in a way that metric is not generated if there is no data. > > Currently metrics start with 0 and it's not possible to distinguish if > > latency is 0 (or close to 0) or there is no data. You can always check > > req_tot or lbconn (depending on mode), but that makes things much more > > complicated thus I decided to only expose those metrics if there is data > > (at least 1 request has been made). Gaps on low-traffic-compare.png graph > > indicate that during that period there were no requests and thus we > return > > no data. > > In fact it's a different metric (though very useful). I've had the same > needs recently. The current ctime reports the avg time experienced by the > last 1024 *requests* and is documented as such, so when you want to think > in terms of user experience it's the one to consider. For example, if you > have a 99% reuse rate and 1% connect rate, even a DC far away at 100ms > will only add 1ms on average to the request time, because 99% of the time, > the connect time really is zero for the request. Ditto if you're using > the cache. Your timer reports the average connect time per *connection*, > and there it's much more suitable to analyse the infrastructure's impact > on performance. Both are equally useful but do not report the same metric. > It was not my intention to change that. In my mind they both report the very same thing with one major difference: current implementation provides misleading results before it processes at least TIME_STATS_SAMPLES samples (it always assumes there were at least TIME_STATS_SAMPLES samples processed) whereas new implementation dynamically scales n value depending on amount of samples it processed so far. In fact up to TIME_STATS_SAMPLES samples it should produce an exact moving average. New implementation takes into account reuse logic too, but after reload there are no connections to be re-used, so that's why latency sharply goes up (I actually looked at timing reported in log entries to confirm that). ctime, ttime etc. are also produced for tcp mode (which makes sense in my mind), so documentation might not be accurate in this matter (just tested it). > I'd be in favor of creating a new one for yours. Anyway we're still missing > a number of other ones like the average TLS handshake cost on each side, > which should also be useful both per connection and per request. I'm saying > this in case that helps figuring a pattern to find a name for this one :-) > I don't mind creating additional metrics if they make sense of course. One of them (which could help to avoid exceptions) would be req_tot for a server (it's available for backend). To dynamically scale n, req_tot is much more suitable than lbconn for http mode (lbconn is incremented much earlier than req_tot creating a time window when metrics can be incorrectly calculated leading to wrong averages). Of course if you still think it's a good idea to separate them (personally I'm not convinced about this unless you don't want to change behaviour of existing metric etc.) I can make new metrics eg. ctime_avg. Keep in mind that I already decided (as you su
Re: QAT intermittent healthcheck errors
Hi Emeric, On 5/13/19 11:06 AM, Emeric Brun wrote: Just to known that I'm waiting for the feedback of intel's team and I will receive QAT 1.7 compliant hardware soon to make some tests here. Thank you for an update. Regards, Marcin Deranek
Re: [External] Re: QAT intermittent healthcheck errors
Hi Emeric, On 5/7/19 1:53 PM, Emeric Brun wrote: On 5/7/19 1:24 PM, Marcin Deranek wrote: Hi Emeric, On 5/7/19 11:44 AM, Emeric Brun wrote: Hi Marcin,>>>>>> As I use HAProxy 1.8 I had to adjust the patch (see attachment for end result). Unfortunately after applying the patch there is no change in behavior: we still leak /dev/usdm_drv descriptors and have "stuck" HAProxy instances after reload.. Regards, Could you perform a test recompiling the usdm_drv and the engine with this patch, it applies on QAT 1.7 but I've no hardware to test this version here. It should fix the fd leak. It did fix fd leak: # ls -al /proc/2565/fd|fgrep dev lr-x-- 1 root root 64 May 7 13:15 0 -> /dev/null lrwx-- 1 root root 64 May 7 13:15 7 -> /dev/usdm_drv # systemctl reload haproxy.service # ls -al /proc/2565/fd|fgrep dev lr-x-- 1 root root 64 May 7 13:15 0 -> /dev/null lrwx-- 1 root root 64 May 7 13:15 8 -> /dev/usdm_drv # systemctl reload haproxy.service # ls -al /proc/2565/fd|fgrep dev lr-x-- 1 root root 64 May 7 13:15 0 -> /dev/null lrwx-- 1 root root 64 May 7 13:15 9 -> /dev/usdm_drv But there are still stuck processes :-( This is with both patches included: for QAT and HAProxy. Regards, Marcin Deranek Thank you Marcin! Anyway it's was also a bug. Could you process a 'show fds' command on a stucked process adding the patch in attachement. I did apply this patch and all previous patches (QAT + HAProxy ssl_free_engine). This is what I got after 1st reload: show proc # 8025master 0 1 0d 00h03m25s # workers 31269 worker 1 0 0d 00h00m39s 31270 worker 2 0 0d 00h00m39s 31271 worker 3 0 0d 00h00m39s 31272 worker 4 0 0d 00h00m39s # old workers 9286worker [was: 1]1 0d 00h03m25s 9287worker [was: 2]1 0d 00h03m25s 9288worker [was: 3]1 0d 00h03m25s 9289worker [was: 4]1 0d 00h03m25s @!9286 show fd 13 : st=0x05(R:PrA W:pra) ev=0x01(heopI) [lc] cache=0 owner=0x23eaae0 iocb=0x4877c0(mworker_accept_wrapper) tmask=0x1 umask=0x0 16 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x4e1ab0 iocb=0x4e1ab0(thread_sync_io_handler) tmask=0x umask=0x0 20 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x1601b840 iocb=0x4f4d50(ssl_async_fd_free) tmask=0x1 umask=0x0 21 : st=0x22(R:pRa W:pRa) ev=0x00(heopi) [lc] cache=0 owner=0x1f0ec4f0 iocb=0x4ce6e0(conn_fd_handler) tmask=0x1 umask=0x0 cflg=0x00241300 fe=GLOBAL mux=PASS mux_ctx=0x22ad8630 1412 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x1bab1f30 iocb=0x4f4d50(ssl_async_fd_free) tmask=0x1 umask=0x0 1413 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x247e5bc0 iocb=0x4f4d50(ssl_async_fd_free) tmask=0x1 umask=0x0 1414 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x18883650 iocb=0x4f4d50(ssl_async_fd_free) tmask=0x1 umask=0x0 1415 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x14476c10 iocb=0x4f4d50(ssl_async_fd_free) tmask=0x1 umask=0x0 1416 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x11a27850 iocb=0x4f4d50(ssl_async_fd_free) tmask=0x1 umask=0x0 1418 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x12008230 iocb=0x4f4d50(ssl_async_fd_free) tmask=0x1 umask=0x0 1419 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x1bb0a570 iocb=0x4f4d50(ssl_async_fd_free) tmask=0x1 umask=0x0 1420 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x11c94790 iocb=0x4f4d50(ssl_async_fd_free) tmask=0x1 umask=0x0 1421 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x1449e050 iocb=0x4f4d50(ssl_async_fd_free) tmask=0x1 umask=0x0 1422 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x1f00c150 iocb=0x4f4d50(ssl_async_fd_free) tmask=0x1 umask=0x0 1423 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x15f40550 iocb=0x4f4d50(ssl_async_fd_free) tmask=0x1 umask=0x0 1424 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x124b6340 iocb=0x4f4d50(ssl_async_fd_free) tmask=0x1 umask=0x0 1425 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x11fe4500 iocb=0x4f4d50(ssl_async_fd_free) tmask=0x1 umask=0x0 1426 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x11c70a60 iocb=0x4f4d50(ssl_async_fd_free) tmask=0x1 umask=0x0 1427 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x12572540 iocb=0x4f4d50(ssl_async_fd_free) tmask=0x1 umask=0x0 1428 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x1249a420 iocb=0x4f4
Re: [External] Re: QAT intermittent healthcheck errors
Hi Emeric, On 5/7/19 11:44 AM, Emeric Brun wrote: Hi Marcin,>>>>>> As I use HAProxy 1.8 I had to adjust the patch (see attachment for end result). Unfortunately after applying the patch there is no change in behavior: we still leak /dev/usdm_drv descriptors and have "stuck" HAProxy instances after reload.. Regards, Could you perform a test recompiling the usdm_drv and the engine with this patch, it applies on QAT 1.7 but I've no hardware to test this version here. It should fix the fd leak. It did fix fd leak: # ls -al /proc/2565/fd|fgrep dev lr-x-- 1 root root 64 May 7 13:15 0 -> /dev/null lrwx-- 1 root root 64 May 7 13:15 7 -> /dev/usdm_drv # systemctl reload haproxy.service # ls -al /proc/2565/fd|fgrep dev lr-x-- 1 root root 64 May 7 13:15 0 -> /dev/null lrwx-- 1 root root 64 May 7 13:15 8 -> /dev/usdm_drv # systemctl reload haproxy.service # ls -al /proc/2565/fd|fgrep dev lr-x-- 1 root root 64 May 7 13:15 0 -> /dev/null lrwx-- 1 root root 64 May 7 13:15 9 -> /dev/usdm_drv But there are still stuck processes :-( This is with both patches included: for QAT and HAProxy. Regards, Marcin Deranek
Re: QAT intermittent healthcheck errors
On 5/7/19 11:44 AM, Emeric Brun wrote: Could you perform a test recompiling the usdm_drv and the engine with this patch, it applies on QAT 1.7 but I've no hardware to test this version here. It should fix the fd leak. Will do and report back. Marcin Deranek
Re: QAT intermittent healthcheck errors
Hi Emeric, On 5/3/19 5:54 PM, Emeric Brun wrote: Hi Marcin, On 5/3/19 4:56 PM, Marcin Deranek wrote: Hi Emeric, On 5/3/19 4:50 PM, Emeric Brun wrote: I've a testing platform here but I don't use the usdm_drv but the qat_contig_mem and I don't reproduce this issue (I'm using QAT 1.5, as the doc says to use with my chip) . I see. I use qat 1.7 and qat-engine 0.5.40. Anyway, could you re-compile a haproxy's binary if I provide you a testing patch? Sure, that should not be a problem. The patch in attachment. As I use HAProxy 1.8 I had to adjust the patch (see attachment for end result). Unfortunately after applying the patch there is no change in behavior: we still leak /dev/usdm_drv descriptors and have "stuck" HAProxy instances after reload.. Regards, Marcin Deranek --- src/haproxy.c.orig 2019-05-06 13:58:39.814399336 +0200 +++ src/haproxy.c 2019-05-06 14:04:31.992043907 +0200 @@ -732,6 +732,12 @@ ptdf->fct(); if (fdtab) deinit_pollers(); +#if defined(USE_OPENSSL) +#ifndef OPENSSL_NO_ENGINE + /* Engines may have opened fds and we must close them */ + ssl_free_engines(); +#endif +#endif /* compute length */ while (next_argv[next_argc])
Re: [External] Re: QAT intermittent healthcheck errors
Hi Emeric, On 5/3/19 4:50 PM, Emeric Brun wrote: I've a testing platform here but I don't use the usdm_drv but the qat_contig_mem and I don't reproduce this issue (I'm using QAT 1.5, as the doc says to use with my chip) . I see. I use qat 1.7 and qat-engine 0.5.40. Anyway, could you re-compile a haproxy's binary if I provide you a testing patch? Sure, that should not be a problem. The idea is to perform a deinit in the master to force a close of those '/dev's at each reload. Perhaps It won't fix our issue but this leak of fd should not be. Hope this will give us at least some more insight.. Regards, Marcin Deranek On 5/3/19 4:21 PM, Marcin Deranek wrote: Hi Emeric, It looks like on every reload master leaks /dev/usdm_drv device: # systemctl restart haproxy.service # ls -la /proc/$(cat haproxy.pid)/fd|fgrep dev lr-x-- 1 root root 64 May 3 15:40 0 -> /dev/null lrwx-- 1 root root 64 May 3 15:40 7 -> /dev/usdm_drv # systemctl reload haproxy.service # ls -la /proc/$(cat haproxy.pid)/fd|fgrep dev lr-x-- 1 root root 64 May 3 15:40 0 -> /dev/null lrwx-- 1 root root 64 May 3 15:40 7 -> /dev/usdm_drv lrwx-- 1 root root 64 May 3 15:40 9 -> /dev/usdm_drv # systemctl reload haproxy.service # ls -la /proc/$(cat haproxy.pid)/fd|fgrep dev lr-x-- 1 root root 64 May 3 15:40 0 -> /dev/null lrwx-- 1 root root 64 May 3 15:40 10 -> /dev/usdm_drv lrwx-- 1 root root 64 May 3 15:40 7 -> /dev/usdm_drv lrwx-- 1 root root 64 May 3 15:40 9 -> /dev/usdm_drv Obviously workers do inherit this from the master. Looking at workers I see the following: * 1st gen: # ls -al /proc/36083/fd|awk '/dev/ {print $NF}'|sort /dev/null /dev/null /dev/qat_adf_ctl /dev/qat_adf_ctl /dev/qat_adf_ctl /dev/qat_dev_processes /dev/uio19 /dev/uio3 /dev/uio35 /dev/usdm_drv * 2nd gen: # ls -al /proc/41637/fd|awk '/dev/ {print $NF}'|sort /dev/null /dev/null /dev/qat_adf_ctl /dev/qat_adf_ctl /dev/qat_adf_ctl /dev/qat_dev_processes /dev/uio23 /dev/uio39 /dev/uio7 /dev/usdm_drv /dev/usdm_drv Looks like only /dev/usdm_drv is leaked. Cheers, Marcin Deranek On 5/3/19 2:22 PM, Emeric Brun wrote: Hi Marcin, On 4/29/19 6:41 PM, Marcin Deranek wrote: Hi Emeric, On 4/29/19 3:42 PM, Emeric Brun wrote: Hi Marcin, I've also a contact at intel who told me to try this option on the qat engine: --disable-qat_auto_engine_init_on_fork/--enable-qat_auto_engine_init_on_fork Disable/Enable the engine from being initialized automatically following a fork operation. This is useful in a situation where you want to tightly control how many instances are being used for processes. For instance if an application forks to start a process that does not utilize QAT currently the default behaviour is for the engine to still automatically get started in the child using up an engine instance. After using this flag either the engine needs to be initialized manually using the engine message: INIT_ENGINE or will automatically get initialized on the first QAT crypto operation. The initialization on fork is enabled by default. I tried to build QAT Engine with disabled auto init, but that did not help. Now I get the following during startup: 2019-04-29T15:13:47.142297+02:00 host1 hapee-lb[16604]: qaeOpenFd:753 Unable to initialize memory file handle /dev/usdm_drv 2019-04-29T15:13:47+02:00 localhost hapee-lb[16611]: 127.0.0.1:60512 [29/Apr/2019:15:13:47.139] vip1/23: SSL handshake failure " INIT_ENGINE or will automatically get initialized on the first QAT crypto operation" Perhaps the init appears "with first qat crypto operation" and is delayed after the fork so if a chroot is configured, it doesn't allow some accesses to /dev. Could you perform a test in that case without chroot enabled in the haproxy config ? Removed chroot and now it initializes properly. Unfortunately reload still causes "stuck" HAProxy process :-( Marcin Deranek Could you check with "ls -l /proc//fd" if the "/dev/" is open multiple times after a reload? Emeric
Re: [External] Re: QAT intermittent healthcheck errors
Hi Emeric, It looks like on every reload master leaks /dev/usdm_drv device: # systemctl restart haproxy.service # ls -la /proc/$(cat haproxy.pid)/fd|fgrep dev lr-x-- 1 root root 64 May 3 15:40 0 -> /dev/null lrwx-- 1 root root 64 May 3 15:40 7 -> /dev/usdm_drv # systemctl reload haproxy.service # ls -la /proc/$(cat haproxy.pid)/fd|fgrep dev lr-x-- 1 root root 64 May 3 15:40 0 -> /dev/null lrwx-- 1 root root 64 May 3 15:40 7 -> /dev/usdm_drv lrwx-- 1 root root 64 May 3 15:40 9 -> /dev/usdm_drv # systemctl reload haproxy.service # ls -la /proc/$(cat haproxy.pid)/fd|fgrep dev lr-x-- 1 root root 64 May 3 15:40 0 -> /dev/null lrwx-- 1 root root 64 May 3 15:40 10 -> /dev/usdm_drv lrwx-- 1 root root 64 May 3 15:40 7 -> /dev/usdm_drv lrwx-- 1 root root 64 May 3 15:40 9 -> /dev/usdm_drv Obviously workers do inherit this from the master. Looking at workers I see the following: * 1st gen: # ls -al /proc/36083/fd|awk '/dev/ {print $NF}'|sort /dev/null /dev/null /dev/qat_adf_ctl /dev/qat_adf_ctl /dev/qat_adf_ctl /dev/qat_dev_processes /dev/uio19 /dev/uio3 /dev/uio35 /dev/usdm_drv * 2nd gen: # ls -al /proc/41637/fd|awk '/dev/ {print $NF}'|sort /dev/null /dev/null /dev/qat_adf_ctl /dev/qat_adf_ctl /dev/qat_adf_ctl /dev/qat_dev_processes /dev/uio23 /dev/uio39 /dev/uio7 /dev/usdm_drv /dev/usdm_drv Looks like only /dev/usdm_drv is leaked. Cheers, Marcin Deranek On 5/3/19 2:22 PM, Emeric Brun wrote: Hi Marcin, On 4/29/19 6:41 PM, Marcin Deranek wrote: Hi Emeric, On 4/29/19 3:42 PM, Emeric Brun wrote: Hi Marcin, I've also a contact at intel who told me to try this option on the qat engine: --disable-qat_auto_engine_init_on_fork/--enable-qat_auto_engine_init_on_fork Disable/Enable the engine from being initialized automatically following a fork operation. This is useful in a situation where you want to tightly control how many instances are being used for processes. For instance if an application forks to start a process that does not utilize QAT currently the default behaviour is for the engine to still automatically get started in the child using up an engine instance. After using this flag either the engine needs to be initialized manually using the engine message: INIT_ENGINE or will automatically get initialized on the first QAT crypto operation. The initialization on fork is enabled by default. I tried to build QAT Engine with disabled auto init, but that did not help. Now I get the following during startup: 2019-04-29T15:13:47.142297+02:00 host1 hapee-lb[16604]: qaeOpenFd:753 Unable to initialize memory file handle /dev/usdm_drv 2019-04-29T15:13:47+02:00 localhost hapee-lb[16611]: 127.0.0.1:60512 [29/Apr/2019:15:13:47.139] vip1/23: SSL handshake failure " INIT_ENGINE or will automatically get initialized on the first QAT crypto operation" Perhaps the init appears "with first qat crypto operation" and is delayed after the fork so if a chroot is configured, it doesn't allow some accesses to /dev. Could you perform a test in that case without chroot enabled in the haproxy config ? Removed chroot and now it initializes properly. Unfortunately reload still causes "stuck" HAProxy process :-( Marcin Deranek Could you check with "ls -l /proc//fd" if the "/dev/" is open multiple times after a reload? Emeric
Re: [External] Re: QAT intermittent healthcheck errors
Hi Emeric, On 4/29/19 3:42 PM, Emeric Brun wrote: Hi Marcin, I've also a contact at intel who told me to try this option on the qat engine: --disable-qat_auto_engine_init_on_fork/--enable-qat_auto_engine_init_on_fork Disable/Enable the engine from being initialized automatically following a fork operation. This is useful in a situation where you want to tightly control how many instances are being used for processes. For instance if an application forks to start a process that does not utilize QAT currently the default behaviour is for the engine to still automatically get started in the child using up an engine instance. After using this flag either the engine needs to be initialized manually using the engine message: INIT_ENGINE or will automatically get initialized on the first QAT crypto operation. The initialization on fork is enabled by default. I tried to build QAT Engine with disabled auto init, but that did not help. Now I get the following during startup: 2019-04-29T15:13:47.142297+02:00 host1 hapee-lb[16604]: qaeOpenFd:753 Unable to initialize memory file handle /dev/usdm_drv 2019-04-29T15:13:47+02:00 localhost hapee-lb[16611]: 127.0.0.1:60512 [29/Apr/2019:15:13:47.139] vip1/23: SSL handshake failure " INIT_ENGINE or will automatically get initialized on the first QAT crypto operation" Perhaps the init appears "with first qat crypto operation" and is delayed after the fork so if a chroot is configured, it doesn't allow some accesses to /dev. Could you perform a test in that case without chroot enabled in the haproxy config ? Removed chroot and now it initializes properly. Unfortunately reload still causes "stuck" HAProxy process :-( Marcin Deranek
Re: QAT intermittent healthcheck errors
Hi Emeric, On 4/29/19 2:47 PM, Emeric Brun wrote: Hi Marcin, On 4/19/19 3:26 PM, Marcin Deranek wrote: Hi Emeric, On 4/18/19 4:35 PM, Emeric Brun wrote: An other interesting trace would be to perform a "show sess" command on a stucked process through the master cli. And also the "show fd" Here it is: show proc # 13409 master 0 1 0d 00h03m30s # workers 15084 worker 1 0 0d 00h03m20s 15085 worker 2 0 0d 00h03m20s 15086 worker 3 0 0d 00h03m20s 15087 worker 4 0 0d 00h03m20s # old workers 13415 worker [was: 1] 1 0d 00h03m30s 13416 worker [was: 2] 1 0d 00h03m30s 13417 worker [was: 3] 1 0d 00h03m30s 13418 worker [was: 4] 1 0d 00h03m30s @!13415 show sess 0x4eee9c0: proto=sockpair ts=0a age=0s calls=1 rq[f=40c0c220h,i=0,an=00h,rx=,wx=,ax=] rp[f=80008000h,i=0,an=00h,rx=,wx=,ax=] s0=[7,8h,fd=20,ex=] s1=[7,4018h,fd=-1,ex=] exp= @!13415 show fd 13 : st=0x05(R:PrA W:pra) ev=0x01(heopI) [lc] cache=0 owner=0x1a74ae0 iocb=0x487760(mworker_accept_wrapper) tmask=0x1 umask=0x0 16 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x4e19f0 iocb=0x4e19f0(thread_sync_io_handler) tmask=0x umask=0x0 20 : st=0x22(R:pRa W:pRa) ev=0x00(heopi) [lc] cache=0 owner=0x4fe1860 iocb=0x4ce620(conn_fd_handler) tmask=0x1 umask=0x0 cflg=0x00241300 fe=GLOBAL mux=PASS mux_ctx=0x47dfd50 87 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x3ec1150 iocb=0x4f5d30(unknown) tmask=0x1 umask=0x0 88 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x3c237d0 iocb=0x4f5d30(unknown) tmask=0x1 umask=0x0 @!13416 show sess 0x48f2990: proto=sockpair ts=0a age=0s calls=1 rq[f=40c0c220h,i=0,an=00h,rx=,wx=,ax=] rp[f=80008000h,i=0,an=00h,rx=,wx=,ax=] s0=[7,8h,fd=20,ex=] s1=[7,4018h,fd=-1,ex=] exp= @!13416 show fd 15 : st=0x05(R:PrA W:pra) ev=0x01(heopI) [lc] cache=0 owner=0x34c1540 iocb=0x487760(mworker_accept_wrapper) tmask=0x1 umask=0x0 16 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x4e19f0 iocb=0x4e19f0(thread_sync_io_handler) tmask=0x umask=0x0 20 : st=0x22(R:pRa W:pRa) ev=0x00(heopi) [lc] cache=0 owner=0x4b3cff0 iocb=0x4ce620(conn_fd_handler) tmask=0x1 umask=0x0 cflg=0x00241300 fe=GLOBAL mux=PASS mux_ctx=0x4f0e510 75 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x3a6b2f0 iocb=0x4f5d30(unknown) tmask=0x1 umask=0x0 76 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x43a34e0 iocb=0x4f5d30(unknown) tmask=0x1 umask=0x0 Marcin Deranek 87,88,75,76 appears to be async engine FDs and should be cleaned. I will dig for that. Thank you. I've also a contact at intel who told me to try this option on the qat engine: --disable-qat_auto_engine_init_on_fork/--enable-qat_auto_engine_init_on_fork Disable/Enable the engine from being initialized automatically following a fork operation. This is useful in a situation where you want to tightly control how many instances are being used for processes. For instance if an application forks to start a process that does not utilize QAT currently the default behaviour is for the engine to still automatically get started in the child using up an engine instance. After using this flag either the engine needs to be initialized manually using the engine message: INIT_ENGINE or will automatically get initialized on the first QAT crypto operation. The initialization on fork is enabled by default. I tried to build QAT Engine with disabled auto init, but that did not help. Now I get the following during startup: 2019-04-29T15:13:47.142297+02:00 host1 hapee-lb[16604]: qaeOpenFd:753 Unable to initialize memory file handle /dev/usdm_drv 2019-04-29T15:13:47+02:00 localhost hapee-lb[16611]: 127.0.0.1:60512 [29/Apr/2019:15:13:47.139] vip1/23: SSL handshake failure Probably engine is not manually initialized after forking. Regards, Marcin Deranek
Re: [External] Re: QAT intermittent healthcheck errors
Hi Emeric, On 4/18/19 4:35 PM, Emeric Brun wrote: An other interesting trace would be to perform a "show sess" command on a stucked process through the master cli. And also the "show fd" Here it is: show proc # 13409 master 0 1 0d 00h03m30s # workers 15084 worker 1 0 0d 00h03m20s 15085 worker 2 0 0d 00h03m20s 15086 worker 3 0 0d 00h03m20s 15087 worker 4 0 0d 00h03m20s # old workers 13415 worker [was: 1]1 0d 00h03m30s 13416 worker [was: 2]1 0d 00h03m30s 13417 worker [was: 3]1 0d 00h03m30s 13418 worker [was: 4]1 0d 00h03m30s @!13415 show sess 0x4eee9c0: proto=sockpair ts=0a age=0s calls=1 rq[f=40c0c220h,i=0,an=00h,rx=,wx=,ax=] rp[f=80008000h,i=0,an=00h,rx=,wx=,ax=] s0=[7,8h,fd=20,ex=] s1=[7,4018h,fd=-1,ex=] exp= @!13415 show fd 13 : st=0x05(R:PrA W:pra) ev=0x01(heopI) [lc] cache=0 owner=0x1a74ae0 iocb=0x487760(mworker_accept_wrapper) tmask=0x1 umask=0x0 16 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x4e19f0 iocb=0x4e19f0(thread_sync_io_handler) tmask=0x umask=0x0 20 : st=0x22(R:pRa W:pRa) ev=0x00(heopi) [lc] cache=0 owner=0x4fe1860 iocb=0x4ce620(conn_fd_handler) tmask=0x1 umask=0x0 cflg=0x00241300 fe=GLOBAL mux=PASS mux_ctx=0x47dfd50 87 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x3ec1150 iocb=0x4f5d30(unknown) tmask=0x1 umask=0x0 88 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x3c237d0 iocb=0x4f5d30(unknown) tmask=0x1 umask=0x0 @!13416 show sess 0x48f2990: proto=sockpair ts=0a age=0s calls=1 rq[f=40c0c220h,i=0,an=00h,rx=,wx=,ax=] rp[f=80008000h,i=0,an=00h,rx=,wx=,ax=] s0=[7,8h,fd=20,ex=] s1=[7,4018h,fd=-1,ex=] exp= @!13416 show fd 15 : st=0x05(R:PrA W:pra) ev=0x01(heopI) [lc] cache=0 owner=0x34c1540 iocb=0x487760(mworker_accept_wrapper) tmask=0x1 umask=0x0 16 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x4e19f0 iocb=0x4e19f0(thread_sync_io_handler) tmask=0x umask=0x0 20 : st=0x22(R:pRa W:pRa) ev=0x00(heopi) [lc] cache=0 owner=0x4b3cff0 iocb=0x4ce620(conn_fd_handler) tmask=0x1 umask=0x0 cflg=0x00241300 fe=GLOBAL mux=PASS mux_ctx=0x4f0e510 75 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x3a6b2f0 iocb=0x4f5d30(unknown) tmask=0x1 umask=0x0 76 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc] cache=0 owner=0x43a34e0 iocb=0x4f5d30(unknown) tmask=0x1 umask=0x0 Marcin Deranek
Re: QAT intermittent healthcheck errors
On 4/18/19 11:06 AM, Emeric Brun wrote: I think you can do that this way: Remove the option httchk (or prefix it by "no": "no option httchk " if it is configured into the defaults section and add the following 2 lines: option tcp-check tcp-check connect This shouldn't perform the handshake but just validate that the port is open. The regular traffic will continue to use the ssl on server side. Enabling TCP checks has the very same effect as disabling them: reload works just fine. Marcin Deranek
Re: QAT intermittent healthcheck errors
Hi Emeric, On 4/12/19 5:26 PM, Emeric Brun wrote: Do you have ssl enabled on the server side? Yes, ssl is on frontend and backend with ssl checks enabled. If it is the case could replace health check with a simple tcp check (without ssl)? What I noticed before that if I (re)start HAProxy and reload immediately no stuck processes are present. If I wait before reloading stuck processes show up. After disabling checks (I still keep ssl enabled for normal traffic) reloads work just fine (tried many time). Do you know how to enable TCP healthchecks while keeping SSL for non-healthcheck requests ? Regarding the show info/lsoff it seems there is no more sessions on client side but remaining ssl jobs (CurrSslConns) and I supsect the health checks to miss a cleanup of their ssl sessions using the QAT. (this is just an assumption) In general instance where I test QAT does not have any "real" client traffic except small amount of healtcheck requests per frontend which are internally handled by HAProxy itself. Still TLS handshake still needs to take place. There are many more backend healthchecks. Looks like your assumption was correct.. Regards, Marcin Deranek On 4/12/19 4:43 PM, Marcin Deranek wrote: Hi Emeric, On 4/10/19 2:20 PM, Emeric Brun wrote: On 4/10/19 1:02 PM, Marcin Deranek wrote: Hi Emeric, Our process limit in QAT configuration is quite high (128) and I was able to run 100+ openssl processes without a problem. According to Joel from Intel problem is in cleanup code - presumably when HAProxy exits and frees up QAT resources. Will try to see if I can get more debug information. I've just take a look. Engines deinit ar called: haproxy/src/ssl_sock.c #ifndef OPENSSL_NO_ENGINE void ssl_free_engines(void) { struct ssl_engine_list *wl, *wlb; /* free up engine list */ list_for_each_entry_safe(wl, wlb, &openssl_engines, list) { ENGINE_finish(wl->e); ENGINE_free(wl->e); LIST_DEL(&wl->list); free(wl); } } #endif ... #ifndef OPENSSL_NO_ENGINE hap_register_post_deinit(ssl_free_engines); #endif I don't know how many haproxy processes you are running but if I describe the complete scenario of processes you may note that we reach a limit: It's very unlikely it's the limit as I lowered number of HAProxy processes (from 10 to 4) while keeping QAT NumProcesses equal 32. HAProxy would have problem with this limit while spawning new instances and not tearing down old ones. In such a case QAT would not be initialized for some HAProxy instances (you would see 1 thread vs 2 thread). About threads read below. - the master sends a signal to older processes, those process will unbind and stop to accept new conns but continue to serve remaining sessions until the end. - new processes are started and immediately and init the engine and accept newconns. - When no more sessions remains on an old process, it calls the deinit function of the engine before exiting What I noticed is that each HAProxy with QAT enabled has 2 threads (LWP) - looks like QAT adds extra thread to the process itself. Would adding extra thread possibly mess up HAProxy termination sequence ? Our setup is to run HAProxy in multi process mode - no threads (or 1 thread per process if you wish). I'm also supposed that old processes are stucked because there is some sessions which never ended, perhaps I'm wrong but a strace on an old process could be interesting to know why those processes are stucked. strace only shows these: [pid 11392] 23:24:43.164619 epoll_wait(4, [pid 11392] 23:24:43.164687 <... epoll_wait resumed> [], 200, 0) = 0 [pid 11392] 23:24:43.164761 epoll_wait(4, [pid 11392] 23:24:43.953203 <... epoll_wait resumed> [], 200, 788) = 0 [pid 11392] 23:24:43.953286 epoll_wait(4, [pid 11392] 23:24:43.953355 <... epoll_wait resumed> [], 200, 0) = 0 [pid 11392] 23:24:43.953419 epoll_wait(4, [pid 11392] 23:24:44.010508 <... epoll_wait resumed> [], 200, 57) = 0 [pid 11392] 23:24:44.010589 epoll_wait(4, There are no connections: stucked process only has UDP socket on random port: [root@externallb-124 ~]# lsof -p 6307|fgrep IPv4 hapee-lb 6307 lbengine 83u IPv4 3598779351 0t0 UDP *:19573 You can also use the 'master CLI' using '-S' and you could check if it remains sessions on those older processes (doc is available in management.txt) Before reload * systemd Main PID: 33515 (hapee-lb) Memory: 1.6G CGroup: /system.slice/hapee-1.8-lb.service ├─33515 /opt/hapee-1.8/sbin/hapee-lb -Ws -f /etc/lb_engine/haproxy.cfg -p /run/hapee-lb.pid -S 127.0.0.1:1234 ├─34858 /opt/hapee-1.8/sbin/hapee-lb -Ws -f /etc/lb_engine/haproxy.cfg -p /run/hapee-lb.pid -S 127.0.0.1:1234 ├─34859 /opt/hapee-1.8/sbin/hapee-lb -Ws
Re: [External] Re: QAT intermittent healthcheck errors
Hi Emeric, On 4/10/19 2:20 PM, Emeric Brun wrote: On 4/10/19 1:02 PM, Marcin Deranek wrote: Hi Emeric, Our process limit in QAT configuration is quite high (128) and I was able to run 100+ openssl processes without a problem. According to Joel from Intel problem is in cleanup code - presumably when HAProxy exits and frees up QAT resources. Will try to see if I can get more debug information. I've just take a look. Engines deinit ar called: haproxy/src/ssl_sock.c #ifndef OPENSSL_NO_ENGINE void ssl_free_engines(void) { struct ssl_engine_list *wl, *wlb; /* free up engine list */ list_for_each_entry_safe(wl, wlb, &openssl_engines, list) { ENGINE_finish(wl->e); ENGINE_free(wl->e); LIST_DEL(&wl->list); free(wl); } } #endif ... #ifndef OPENSSL_NO_ENGINE hap_register_post_deinit(ssl_free_engines); #endif I don't know how many haproxy processes you are running but if I describe the complete scenario of processes you may note that we reach a limit: It's very unlikely it's the limit as I lowered number of HAProxy processes (from 10 to 4) while keeping QAT NumProcesses equal 32. HAProxy would have problem with this limit while spawning new instances and not tearing down old ones. In such a case QAT would not be initialized for some HAProxy instances (you would see 1 thread vs 2 thread). About threads read below. - the master sends a signal to older processes, those process will unbind and stop to accept new conns but continue to serve remaining sessions until the end. - new processes are started and immediately and init the engine and accept newconns. - When no more sessions remains on an old process, it calls the deinit function of the engine before exiting What I noticed is that each HAProxy with QAT enabled has 2 threads (LWP) - looks like QAT adds extra thread to the process itself. Would adding extra thread possibly mess up HAProxy termination sequence ? Our setup is to run HAProxy in multi process mode - no threads (or 1 thread per process if you wish). I'm also supposed that old processes are stucked because there is some sessions which never ended, perhaps I'm wrong but a strace on an old process could be interesting to know why those processes are stucked. strace only shows these: [pid 11392] 23:24:43.164619 epoll_wait(4, [pid 11392] 23:24:43.164687 <... epoll_wait resumed> [], 200, 0) = 0 [pid 11392] 23:24:43.164761 epoll_wait(4, [pid 11392] 23:24:43.953203 <... epoll_wait resumed> [], 200, 788) = 0 [pid 11392] 23:24:43.953286 epoll_wait(4, [pid 11392] 23:24:43.953355 <... epoll_wait resumed> [], 200, 0) = 0 [pid 11392] 23:24:43.953419 epoll_wait(4, [pid 11392] 23:24:44.010508 <... epoll_wait resumed> [], 200, 57) = 0 [pid 11392] 23:24:44.010589 epoll_wait(4, There are no connections: stucked process only has UDP socket on random port: [root@externallb-124 ~]# lsof -p 6307|fgrep IPv4 hapee-lb 6307 lbengine 83u IPv4 3598779351 0t0 UDP *:19573 You can also use the 'master CLI' using '-S' and you could check if it remains sessions on those older processes (doc is available in management.txt) Before reload * systemd Main PID: 33515 (hapee-lb) Memory: 1.6G CGroup: /system.slice/hapee-1.8-lb.service ├─33515 /opt/hapee-1.8/sbin/hapee-lb -Ws -f /etc/lb_engine/haproxy.cfg -p /run/hapee-lb.pid -S 127.0.0.1:1234 ├─34858 /opt/hapee-1.8/sbin/hapee-lb -Ws -f /etc/lb_engine/haproxy.cfg -p /run/hapee-lb.pid -S 127.0.0.1:1234 ├─34859 /opt/hapee-1.8/sbin/hapee-lb -Ws -f /etc/lb_engine/haproxy.cfg -p /run/hapee-lb.pid -S 127.0.0.1:1234 ├─34860 /opt/hapee-1.8/sbin/hapee-lb -Ws -f /etc/lb_engine/haproxy.cfg -p /run/hapee-lb.pid -S 127.0.0.1:1234 └─34861 /opt/hapee-1.8/sbin/hapee-lb -Ws -f /etc/lb_engine/haproxy.cfg -p /run/hapee-lb.pid -S 127.0.0.1:1234 * master CLI show proc # 33515 master 0 0 0d 00h00m31s # workers 34858 worker 1 0 0d 00h00m31s 34859 worker 2 0 0d 00h00m31s 34860 worker 3 0 0d 00h00m31s 34861 worker 4 0 0d 00h00m31s After reload: * systemd Main PID: 33515 (hapee-lb) Memory: 3.1G CGroup: /system.slice/hapee-1.8-lb.service ├─33515 /opt/hapee-1.8/sbin/hapee-lb -Ws -f /etc/lb_engine/haproxy.cfg -p /run/hapee-lb.pid -S 127.0.0.1:1234 -sf 34858 34859 34860 34861 -x /run/lb_engine/process-1.sock ├─34858 /opt/hapee-1.8/sbin/hapee-lb -Ws -f /etc/lb_engine/haproxy.cfg -p /run/hapee-lb.pid -S 127.0.0.1:1234 ├─34859 /opt/hapee-1.8/sbin/hapee-lb -Ws -f /etc/lb_engine/hapr
Re: [External] Re: QAT intermittent healthcheck errors
Hi Emeric, Our process limit in QAT configuration is quite high (128) and I was able to run 100+ openssl processes without a problem. According to Joel from Intel problem is in cleanup code - presumably when HAProxy exits and frees up QAT resources. Will try to see if I can get more debug information. Regards, Marcin Deranek On 4/9/19 5:17 PM, Emeric Brun wrote: Hi Marcin, On 4/9/19 3:07 PM, Marcin Deranek wrote: Hi Emeric, I have followed all instructions and I got to the point where HAProxy starts and does the job using QAT (backend healthchecks work and I frontend can provide content over HTTPS). The problems starts when HAProxy gets reloaded. With our current configuration on reload old HAProxy processes do not exit, so after reload you end up with 2 generations of HAProxy processes: before reload and after reload. I tried to find out what are conditions in which HAProxy processes get "stuck" and I was not able to replicate it consistently. In one case it was related to amount of backend servers with 'ssl' on their line, but trying to add 'ssl' to some other servers in other place had no effect. Interestingly in some cases for example with simple configuration (1 frontend + 1 backend) HAProxy produced errors on reload (see attachment) - in those cases processes rarely got "stuck" even though errors were present. /dev/qat_adf_ctl is group writable for the group HAProxy runs on. Any help to get this fixed / resolved would be welcome. Regards, Marcin Deranek I've checked the errors.txt and all the messages were written by the engine and are not part of the haproxy code. I can only do supposition for now but I think we face a first error due to a limitation of the amount of processes trying to access the engine: the reload will double the number of processes trying to attach the engine. Perhaps this issue can be bypassed tweaking the qat configuration file (some advise, from intel would be wellcome). For the old stucked processes: I think the grow of processes also triggers errors on already attached ones in the qat engine but currently I ignore the way this errors are/should be raised to the application, it appears that they are currently not handled and that's why processes would be stuck (sessions may appear still valid for haproxy so the old process continues to wait for their end). We expected they were raised by the openssl API but it appears to not be the case. We have to check if we miss to handle an error polling events on the file descriptor used to communicate with engine. So we have to dig deeper and any help from Intel's guy or Qat aware devs will be appreciate. Emeric
Re: QAT intermittent healthcheck errors
Hi Emeric, I have followed all instructions and I got to the point where HAProxy starts and does the job using QAT (backend healthchecks work and I frontend can provide content over HTTPS). The problems starts when HAProxy gets reloaded. With our current configuration on reload old HAProxy processes do not exit, so after reload you end up with 2 generations of HAProxy processes: before reload and after reload. I tried to find out what are conditions in which HAProxy processes get "stuck" and I was not able to replicate it consistently. In one case it was related to amount of backend servers with 'ssl' on their line, but trying to add 'ssl' to some other servers in other place had no effect. Interestingly in some cases for example with simple configuration (1 frontend + 1 backend) HAProxy produced errors on reload (see attachment) - in those cases processes rarely got "stuck" even though errors were present. /dev/qat_adf_ctl is group writable for the group HAProxy runs on. Any help to get this fixed / resolved would be welcome. Regards, Marcin Deranek On 3/13/19 12:04 PM, Emeric Brun wrote: Hi Marcin, On 3/11/19 4:27 PM, Marcin Deranek wrote: On 3/11/19 11:51 AM, Emeric Brun wrote: Mode async is enabled on both sides, server and frontend side. But on server side, haproxy is using session resuming, so there is a new key computation (full handshake with RSA/DSA computation) only every 5 minutes (openssl default value). You can force to recompute each time setting "no-ssl-reuse" on server line, but it will add a heavy load for ssl computation on the server. Indeed, setting no-ssl-reuse makes use of QAT for healthchecks. Looks like finally we are ready for QAT testing. Thank you Emeric. Regards, Marcin Deranek I've just re-check and i think you should also enable the 'PKEY_CRYPTO' algo to the engine ssl-engine qat algo RSA,DSA,EC,DH,PKEY_CRYPTO It will enable rhe offloading of the TLS1-PRF you can see there: # /opt/booking-openssl/bin/openssl engine -c qat (qat) Reference implementation of QAT crypto engine [RSA, DSA, DH, AES-128-CBC-HMAC-SHA1, AES-128-CBC-HMAC-SHA256, AES-256-CBC-HMAC-SHA1, AES-256-CBC-HMAC-SHA256, TLS1-PRF] R, Emeric 2019-04-09T14:22:45.523342+02:00 externallb hapee-lb[60816]: [NOTICE] 098/142244 (60816) : New worker #1 (61249) forked 2019-04-09T14:22:45.523368+02:00 externallb hapee-lb[60816]: [NOTICE] 098/142244 (60816) : New worker #2 (61250) forked 2019-04-09T14:22:45.523393+02:00 externallb hapee-lb[60816]: [NOTICE] 098/142244 (60816) : New worker #3 (61251) forked 2019-04-09T14:22:45.523418+02:00 externallb hapee-lb[60816]: [NOTICE] 098/142244 (60816) : New worker #4 (61252) forked 2019-04-09T14:22:45.523444+02:00 externallb hapee-lb[60816]: [NOTICE] 098/142244 (60816) : New worker #5 (61253) forked 2019-04-09T14:22:45.523469+02:00 externallb hapee-lb[60816]: [NOTICE] 098/142244 (60816) : New worker #6 (61255) forked 2019-04-09T14:22:45.523493+02:00 externallb hapee-lb[60816]: [NOTICE] 098/142244 (60816) : New worker #7 (61258) forked 2019-04-09T14:22:45.523518+02:00 externallb hapee-lb[60816]: [NOTICE] 098/142244 (60816) : New worker #8 (61259) forked 2019-04-09T14:22:45.523548+02:00 externallb hapee-lb[60816]: [NOTICE] 098/142244 (60816) : New worker #9 (61261) forked 2019-04-09T14:22:45.523596+02:00 externallb hapee-lb[60816]: [error] cpaCyStopInstance() - : Can not get instance info 2019-04-09T14:22:45.523623+02:00 externallb hapee-lb[60816]: [error] SalCtrl_GetEnabledServices() - : Failed to get enabled services from ADF 2019-04-09T14:22:45.523649+02:00 externallb hapee-lb[60816]: [error] SalCtrl_ServiceEventHandler() - : Failed to get enabled services 2019-04-09T14:22:45.523674+02:00 externallb hapee-lb[60816]: [error] SalCtrl_GetEnabledServices() - : Failed to get enabled services from ADF 2019-04-09T14:22:45.523699+02:00 externallb hapee-lb[60816]: [error] SalCtrl_ServiceEventHandler() - : Failed to get enabled services 2019-04-09T14:22:45.523724+02:00 externallb hapee-lb[60816]: [error] SalCtrl_GetEnabledServices() - : Failed to get enabled services from ADF 2019-04-09T14:22:45.523749+02:00 externallb hapee-lb[60816]: [error] SalCtrl_ServiceEventHandler() - : Failed to get enabled services 2019-04-09T14:22:45.523774+02:00 externallb hapee-lb[60816]: [error] SalCtrl_GetEnabledServices() - : Failed to get enabled services from ADF 2019-04-09T14:22:45.523799+02:00 externallb hapee-lb[60816]: [error] SalCtrl_ServiceEventHandler() - : Failed to get enabled services 2019-04-09T14:22:45.523823+02:00 externallb hapee-lb[60816]: [error] SalCtrl_GetEnabledServices() - : Failed to get enabled services from ADF 2019-04-09T14:22:45.523848+02:00 externallb hapee-lb[60816]: [error] SalCtrl_ServiceEventHandler() - : Failed to get enabled services 2019-04-09T14:22:45.523874+02:00 externallb hapee-lb[60816]: [error] SalCtrl_GetEnabledServices() - : Failed
Re: [External] Re: QAT intermittent healthcheck errors
Hi Emeric, On 3/11/19 2:48 PM, Emeric Brun wrote: Once again, you could add the "no-ssl-reuse" statement if you want to check if QAT offloads the backend side, but it is clearly not an optimal option for production because it will generate an heavy load on your servers and force them to recompute keys for each connections. I just wanted to make sure that QAT is involved in both and does what it suppose to do based on data rather than hope or trust :-)) We won't be running it with no-ssl-reuse as for obvious reasons we don't want to make more load than necessary. Thank you once again for your help. Regards, Marcin Deranek
Re: [External] Re: QAT intermittent healthcheck errors
On 3/11/19 11:51 AM, Emeric Brun wrote: Mode async is enabled on both sides, server and frontend side. But on server side, haproxy is using session resuming, so there is a new key computation (full handshake with RSA/DSA computation) only every 5 minutes (openssl default value). You can force to recompute each time setting "no-ssl-reuse" on server line, but it will add a heavy load for ssl computation on the server. Indeed, setting no-ssl-reuse makes use of QAT for healthchecks. Looks like finally we are ready for QAT testing. Thank you Emeric. Regards, Marcin Deranek
Re: QAT intermittent healthcheck errors
Hi Emeric, On 3/8/19 11:24 AM, Emeric Brun wrote: Are you sure that servers won't use ECDSA certificates? Do you check that conn are successful forcing 'ECDHE-RSA-AES256-GCM-SHA384' Backend servers only support TLS 1.2 and RSA certificates. Could you check algo supported by QAT doing this ?: openssl engine -c qat # /opt/booking-openssl/bin/openssl engine -c qat (qat) Reference implementation of QAT crypto engine [RSA, DSA, DH, AES-128-CBC-HMAC-SHA1, AES-128-CBC-HMAC-SHA256, AES-256-CBC-HMAC-SHA1, AES-256-CBC-HMAC-SHA256, TLS1-PRF] Could you retry with this config: ssl-engine qat algo RSA,DSA,EC,DH Just did that and experienced the very same effect: no QAT activity for backend server healthchecks :-( When I add frontend eg. frontend frontend1 bind 127.0.0.1:8443 ssl crt /etc/lb_engine/data/generated/ssl/10.252.24.7:443 default_backend pool_all and make some connections/requests (TLS1.2 and/or TLS/1.3) to the frontend I see QAT activity, but *NO* activity when HAProxy is "idle" (only doing healthchecks to backend servers: TLS 1.2 only). This feels like healthchecks are not passing through QAT engine for whatever reason :-( Even enabling HTTP check for the backend (option httpchk) does not make any difference. The question: Is SSL Async Mode actually supported on the backend side (either healthchecks and/or normal traffic) ? Regards, Marcin Deranek
Re: [External] Re: QAT intermittent healthcheck errors
Hi Emeric, On 3/8/19 4:43 PM, Emeric Brun wrote: I've just realized that if your server are TLSv1.3 ssl-default-server-ciphers won't force anything (see ssl-default-server-ciphersuites documentation) Backend servers are 'only' TLS 1.2, so it should have desired effect. Will test suggested configuration changes and report shortly. Marcin Deranek
Re: QAT intermittent healthcheck errors
Hi, On 3/6/19 6:36 PM, Emeric Brun wrote: According to the documentation: ssl-mode-async Adds SSL_MODE_ASYNC mode to the SSL context. This enables asynchronous TLS I/O operations if asynchronous capable SSL engines are used. The current implementation supports a maximum of 32 engines. The Openssl ASYNC API doesn't support moving read/write buffers and is not compliant with haproxy's buffer management. So the asynchronous mode is disabled on read/write operations (it is only enabled during initial and reneg handshakes). Asynchronous mode is disabled on the read/write operation and is only enabled during handshake. It means that for the ciphering process the engine will be used in blocking mode (not async) which could result to unpredictable behavior on timers because the haproxy process will sporadically fully blocked waiting for the engine. To avoid this issue, you should ensure to use QAT only for the asymmetric computing algorithm (such as RSA DSA ECDSA). and not for ciphering ones (AES and everything else ...) I did explicitly enabled RSA algos: ssl-engine qat algo RSA and errors were gone at that point. Unfortunately all QAT activity too as /sys/kernel/debug/qat_c6xx_\:0*/fw_counters were reporting identical values (previously they were incrementing). I did explicitly enforce RSA: ssl-default-server-ciphers ECDHE-RSA-AES256-GCM-SHA384 but that did not help. Do I miss something ? Regards, Marcin Deranek
Re: QAT intermittent healthcheck errors
Hi, On 3/6/19 6:36 PM, Emeric Brun wrote: To avoid this issue, you should ensure to use QAT only for the asymmetric computing algorithm (such as RSA DSA ECDSA). and not for ciphering ones (AES and everything else ...) The ssl engine statement allow you to filter such algos: ssl-engine [algo ] I'm pretty sure I tried this, but I will try to re-test again with eg. RSA specified and see if that makes any difference. Regards, Marcin Deranek
QAT intermittent healthcheck errors
Hi, In a process of evaluating performance of Intel Quick Assist Technology in conjunction with HAProxy software I acquired Intel C62x Chipset card for testing. I configured QAT engine in the following manner: * /etc/qat/c6xx_dev[012].conf [GENERAL] ServicesEnabled = cy ConfigVersion = 2 CyNumConcurrentSymRequests = 512 CyNumConcurrentAsymRequests = 64 statsGeneral = 1 statsDh = 1 statsDrbg = 1 statsDsa = 1 statsEcc = 1 statsKeyGen = 1 statsDc = 1 statsLn = 1 statsPrime = 1 statsRsa = 1 statsSym = 1 KptEnabled = 0 StorageEnabled = 0 PkeServiceDisabled = 0 DcIntermediateBufferSizeInKB = 64 [KERNEL] NumberCyInstances = 0 NumberDcInstances = 0 [SHIM] NumberCyInstances = 1 NumberDcInstances = 0 NumProcesses = 16 LimitDevAccess = 0 Cy0Name = "UserCY0" Cy0IsPolled = 1 Cy0CoreAffinity = 0 OpenSSL produces good results without warnings / errors: * No QAT involved $ openssl speed -elapsed rsa2048 You have chosen to measure elapsed time instead of user CPU time. Doing 2048 bits private rsa's for 10s: 10858 2048 bits private RSA's in 10.00s Doing 2048 bits public rsa's for 10s: 361207 2048 bits public RSA's in 10.00s OpenSSL 1.1.1a FIPS 20 Nov 2018 built on: Tue Jan 22 20:43:41 2019 UTC options:bn(64,64) md2(char) rc4(16x,int) des(int) aes(partial) idea(int) blowfish(ptr) compiler: gcc -fPIC -pthread -m64 -Wa,--noexecstack -Wall -O3 -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -Wa,--noexecstack -DOPENSSL_USE_NODELETE -DL_ENDIAN -DOPENSSL_PIC -DOPENSSL_CPUID_OBJ -DOPENSSL_IA32_SSE2 -DOPENSSL_BN_ASM_MONT -DOPENSSL_BN_ASM_MONT5 -DOPENSSL_BN_ASM_GF2m -DSHA1_ASM -DSHA256_ASM -DSHA512_ASM -DKECCAK1600_ASM -DRC4_ASM -DMD5_ASM -DAES_ASM -DVPAES_ASM -DBSAES_ASM -DGHASH_ASM -DECP_NISTZ256_ASM -DX25519_ASM -DPADLOCK_ASM -DPOLY1305_ASM -DZLIB -DNDEBUG -DPURIFY -DDEVRANDOM="\"/dev/urandom\"" -DSYSTEM_CIPHERS_FILE="/opt/openssl/etc/crypto-policies/back-ends/openssl.config" signverifysign/s verify/s rsa 2048 bits 0.000921s 0.28s 1085.8 36120.7 * QAT enabled $ openssl speed -elapsed -engine qat -async_jobs 32 rsa2048 engine "qat" set. You have chosen to measure elapsed time instead of user CPU time. Doing 2048 bits private rsa's for 10s: 205425 2048 bits private RSA's in 10.00s Doing 2048 bits public rsa's for 10s: 2150270 2048 bits public RSA's in 10.00s OpenSSL 1.1.1a FIPS 20 Nov 2018 built on: Tue Jan 22 20:43:41 2019 UTC options:bn(64,64) md2(char) rc4(16x,int) des(int) aes(partial) idea(int) blowfish(ptr) compiler: gcc -fPIC -pthread -m64 -Wa,--noexecstack -Wall -O3 -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -Wa,--noexecstack -DOPENSSL_USE_NODELETE -DL_ENDIAN -DOPENSSL_PIC -DOPENSSL_CPUID_OBJ -DOPENSSL_IA32_SSE2 -DOPENSSL_BN_ASM_MONT -DOPENSSL_BN_ASM_MONT5 -DOPENSSL_BN_ASM_GF2m -DSHA1_ASM -DSHA256_ASM -DSHA512_ASM -DKECCAK1600_ASM -DRC4_ASM -DMD5_ASM -DAES_ASM -DVPAES_ASM -DBSAES_ASM -DGHASH_ASM -DECP_NISTZ256_ASM -DX25519_ASM -DPADLOCK_ASM -DPOLY1305_ASM -DZLIB -DNDEBUG -DPURIFY -DDEVRANDOM="\"/dev/urandom\"" -DSYSTEM_CIPHERS_FILE="/opt/openssl/etc/crypto-policies/back-ends/openssl.config" signverifysign/s verify/s rsa 2048 bits 0.49s 0.05s 20542.5 215027.0 So far so good. Unfortunately HAProxy 1.8 iwth QAT engine enabled periodically fail with SSL checks of backend servers. The simplest configuration I could get to reproduce it: * /etc/haproxy/haproxy.cfg global user lbengine group lbengine daemon ssl-mode-async ssl-engine qat ssl-server-verify none stats socket /run/lb_engine/process-1.sock user lbengine group lbengine mode 660 level admin expose-fd listeners process 1 defaults mode http timeout check 5s timeout connect 4s backend pool_all default-server inter 5s server server1 ip1:443 check ssl server server2 ip2:443 check ssl ... server serverN ipN:443 check ssl Without QAT enabled everything works just fine - healthchecks do not flap. With QAT engine enabled random server healtchecks flap: they fail and then shortly after they recover eg. 2019-03-06T15:06:22+01:00 localhost hapee-lb[1832]: Server pool_all/server1 is DOWN, reason: Layer6 timeout, check duration: 4000ms. 110 active and 0 backup servers left. 0 sessions active, 0 requeued, 0 remaining in queue. 2019-03-06T15:06:32+01:00 localhost hapee-lb[1832]: Server pool_all/server1 is UP, reason: Layer6 check passed, check duration: 13ms. 117 active and 0 backup servers online. 0 sessions requeued, 0 total in queue. Increasing check frequency (lowering check interval) makes the problem occur more frequently. Anybody has a clue why this is happening ? Has anybody seen such behavior ? Regards, Marcin Deranek
Re: [External] Re: [PATCH] MEDIUM: sample: Extend functionality for field/word converters
On 04/17/2018 11:29 AM, Willy Tarreau wrote: > Pretty cool, thank you. I've already missed this for domain names (using > the dot as the delimitor). Now merged :-) Cool. We missed this functionality too, so eventually I invested some time to get it done :-)) Marcin Deranek
[PATCH] MEDIUM: sample: Extend functionality for field/word converters
Hi, This patch extends functionality of field/word converters, so it's possible to count fields/words from the beginning/end of the string and extract multiple fields/words if needed. Change is backward compatible and should cleanly apply to both 1.8 & 1.9 branches. Regards, Marcin Deranek >From 3393f952788e26e6e8add5b6ca472d3e765b57ca Mon Sep 17 00:00:00 2001 From: Marcin Deranek Date: Mon, 16 Apr 2018 14:30:46 +0200 Subject: [PATCH] Extend functionality for field/word converters Extend functionality of field/word converters, so it's possible to extract field(s)/word(s) counting from the beginning/end and/or extract multiple fields/words (including separators) eg. str(f1_f2_f3__f5),field(2,_,2) # f2_f3 str(f1_f2_f3__f5),field(2,_,0) # f2_f3__f5 str(f1_f2_f3__f5),field(-2,_,3) # f2_f3_ str(f1_f2_f3__f5),field(-3,_,0) # f1_f2_f3 str(w1_w2_w3___w4),word(3,_,2) # w3___w4 str(w1_w2_w3___w4),word(2,_,0) # w2_w3___w4 str(w1_w2_w3___w4),word(-2,_,3) # w1_w2_w3 str(w1_w2_w3___w4),word(-3,_,0) # w1_w2 Change is backward compatible. --- doc/configuration.txt | 34 ++--- src/sample.c | 132 +- 2 files changed, 125 insertions(+), 41 deletions(-) diff --git a/doc/configuration.txt b/doc/configuration.txt index 1e0b26f8..a9f75579 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -12907,10 +12907,20 @@ even Returns a boolean TRUE if the input value of type signed integer is even otherwise returns FALSE. It is functionally equivalent to "not,and(1),bool". -field(,) - Extracts the substring at the given index considering given delimiters from - an input string. Indexes start at 1 and delimiters are a string formatted - list of chars. +field(,[,]) + Extracts the substring at the given index counting from the beginning + (positive index) or from the end (negative index) considering given delimiters + from an input string. Indexes start at 1 or -1 and delimiters are a string + formatted list of chars. Optionally you can specify of fields to + extract (default: 1). Value of 0 indicates extraction of all remaining + fields. + + Example : + str(f1_f2_f3__f5),field(5,_)# f5 + str(f1_f2_f3__f5),field(2,_,0) # f2_f3__f5 + str(f1_f2_f3__f5),field(2,_,2) # f2_f3 + str(f1_f2_f3__f5),field(-2,_,3) # f2_f3_ + str(f1_f2_f3__f5),field(-3,_,0) # f1_f2_f3 hex Converts a binary input sample to a hex string containing two hex digits per @@ -13440,9 +13450,19 @@ utime([,]) # e.g. 20140710162350 127.0.0.1:57325 log-format %[date,utime(%Y%m%d%H%M%S)]\ %ci:%cp -word(,) - Extracts the nth word considering given delimiters from an input string. - Indexes start at 1 and delimiters are a string formatted list of chars. +word(,[,]) + Extracts the nth word counting from the beginning (positive index) or from + the end (negative index) considering given delimiters from an input string. + Indexes start at 1 or -1 and delimiters are a string formatted list of chars. + Optionally you can specify of words to extract (default: 1). + Value of 0 indicates extraction of all remaining words. + + Example : + str(f1_f2_f3__f5),word(4,_)# f5 + str(f1_f2_f3__f5),word(2,_,0) # f2_f3__f5 + str(f1_f2_f3__f5),word(3,_,2) # f3__f5 + str(f1_f2_f3__f5),word(-2,_,3) # f1_f2_f3 + str(f1_f2_f3__f5),word(-3,_,0) # f1_f2 wt6([]) Hashes a binary input sample into an unsigned 32-bit quantity using the WT6 diff --git a/src/sample.c b/src/sample.c index 71ee59f0..154beb5c 100644 --- a/src/sample.c +++ b/src/sample.c @@ -1997,27 +1997,54 @@ static int sample_conv_field_check(struct arg *args, struct sample_conv *conv, */ static int sample_conv_field(const struct arg *arg_p, struct sample *smp, void *private) { - unsigned int field; + int field; char *start, *end; int i; + int count = (arg_p[2].type == ARGT_SINT) ? arg_p[2].data.sint : 1; if (!arg_p[0].data.sint) return 0; - field = 1; - end = start = smp->data.u.str.str; - while (end - smp->data.u.str.str < smp->data.u.str.len) { - - for (i = 0 ; i < arg_p[1].data.str.len ; i++) { - if (*end == arg_p[1].data.str.str[i]) { -if (field == arg_p[0].data.sint) - goto found; -start = end+1; -field++; -break; + if (arg_p[0].data.sint < 0) { + field = -1; + end = start = smp->data.u.str.str + smp->data.u.str.len; + while (start > smp->data.u.str.str) { + for (i = 0 ; i < arg_p[1].data.str.len ; i++) { +if (*(start-1) == arg_p[1].data.str.str[i]) { + if (field == arg_p[0].data.sint) { + if (count == 1) + goto found; + else if (count > 1) + count--; + } else { + end = start-1; + field--; + } + break; +} } + start--; + } + } else { + field = 1; + end = start = smp->data.u.str.str; + while (end - smp->data.u.str.str < smp->data.u.str.len) { +
MINOR: proxy: Add fe_defbe fetcher
Hi, New fetcher which adds ability to retrieve default backend name for frontend. Should cleanly apply to both 1.8 & 1.9 branches. Regards, Marcin Deranek >From b2eeddea4859ab247b5315fb470706175797d434 Mon Sep 17 00:00:00 2001 From: Marcin Deranek Date: Fri, 13 Apr 2018 14:37:50 +0200 Subject: [PATCH] MINOR: proxy: Add fe_defbe fetcher Patch adds ability to fetch frontend's default backend name in your logic, so it can be used later to derive other backend names to make routing decisions. --- doc/configuration.txt | 4 src/frontend.c| 17 + 2 files changed, 21 insertions(+) diff --git a/doc/configuration.txt b/doc/configuration.txt index 43c91114..1f44b8e3 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -13927,6 +13927,10 @@ fe_name : string backends to check from which frontend it was called, or to stick all users coming via a same frontend to the same server. +fe_defbe : string + Returns a string containing the frontend's default backend name. It can be + used in frontends to check which backend will handle requests by default. + sc_bytes_in_rate([,]) : integer sc0_bytes_in_rate([]) : integer sc1_bytes_in_rate([]) : integer diff --git a/src/frontend.c b/src/frontend.c index 94d90f59..d935037d 100644 --- a/src/frontend.c +++ b/src/frontend.c @@ -194,6 +194,22 @@ smp_fetch_fe_name(const struct arg *args, struct sample *smp, const char *kw, vo return 1; } +/* set string to the name of the default backend */ +static int +smp_fetch_fe_defbe(const struct arg *args, struct sample *smp, const char *kw, void *private) +{ + if (!smp->sess->fe->defbe.be) + return 0; + smp->data.u.str.str = (char *)smp->sess->fe->defbe.be->id; + if (!smp->data.u.str.str) + return 0; + + smp->data.type = SMP_T_STR; + smp->flags = SMP_F_CONST; + smp->data.u.str.len = strlen(smp->data.u.str.str); + return 1; +} + /* set temp integer to the number of HTTP requests per second reaching the frontend. * Accepts exactly 1 argument. Argument is a frontend, other types will cause * an undefined behaviour. @@ -241,6 +257,7 @@ static struct sample_fetch_kw_list smp_kws = {ILH, { { "fe_conn", smp_fetch_fe_conn, ARG1(1,FE), NULL, SMP_T_SINT, SMP_USE_INTRN, }, { "fe_id",smp_fetch_fe_id,0, NULL, SMP_T_SINT, SMP_USE_FTEND, }, { "fe_name", smp_fetch_fe_name, 0, NULL, SMP_T_STR, SMP_USE_FTEND, }, + { "fe_defbe", smp_fetch_fe_defbe, 0, NULL, SMP_T_STR, SMP_USE_FTEND, }, { "fe_req_rate", smp_fetch_fe_req_rate, ARG1(1,FE), NULL, SMP_T_SINT, SMP_USE_INTRN, }, { "fe_sess_rate", smp_fetch_fe_sess_rate, ARG1(1,FE), NULL, SMP_T_SINT, SMP_USE_INTRN, }, { /* END */ }, -- 2.16.1
Re: [External] Re: [PATCH] nbsrv() behavior when backend is disabled
Hi Willy, On 12/23/2016 12:10 AM, Willy Tarreau wrote: > > Hmmm I think that makes sense indeed. Just out of curiosity in what > situation did you encounter this condition ? A reload maybe ? We have some business logic which tries to detect if there are any usable members in the backend before using it. For that purpose we make use of nbsrv() fetcher. Unfortunately disabling backend (useful when you want to quickly get traffic away from those servers) has no impact on nbsrv() output. > > Thanks. I've copied your analysis above as the commit message and adjusted > the subject to mention it's a minor bug in the backend part. I'm tagging it > for backport down to 1.5. > Thank you. Marcin
[PATCH] nbsrv() behavior when backend is disabled
Hi, According to nbsrv() documentation this fetcher should return "an integer value corresponding to the number of usable servers". In case backend is disabled none of servers is usable, so I believe fetcher should return 0. Attached patch addresses such condition. Regards, Marcin Deranek >From 5b3eeb5d9a214d880773c03e0213a62772f7b271 Mon Sep 17 00:00:00 2001 From: Marcin Deranek Date: Thu, 22 Dec 2016 16:21:08 +0100 Subject: [PATCH] nbsrv() should return 0 if backend is disabled X-Bogosity: Ham, tests=bogofilter, spamicity=0.00, version=1.2.4 --- src/backend.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/backend.c b/src/backend.c index 57f811f..69e2c90 100644 --- a/src/backend.c +++ b/src/backend.c @@ -1608,7 +1608,9 @@ smp_fetch_nbsrv(const struct arg *args, struct sample *smp, const char *kw, void smp->data.type = SMP_T_SINT; px = args->data.prx; - if (px->srv_act) + if (px->state == PR_STSTOPPED) + smp->data.u.sint = 0; + else if (px->srv_act) smp->data.u.sint = px->srv_act; else if (px->lbprm.fbck) smp->data.u.sint = 1; -- 2.10.2
Re: [PATCH] Add fe_name/be_name fetchers next to existing fe_id/be_id
On 12/13/2016 11:25 AM, Willy Tarreau wrote: >> I think we'll definitely need to have some string-oriented operators. We >> could implement them using converters (eg: append and compare mostly), but >> we also thought that we could replace every sample fetch with a converter >> and allow all expressions to be simplified (eg: a fetcher would just be >> like a converter except that it doesn't affect the existing stack and >> pushes a new value). >> >> With all that in mind, I'm not much tempted to implement too many things >> if we have to redesign them a short time later! Makes sense. Any ETA when this is going to arrive ? >> Yes sure :-) Feel free to send a patch regarding this otherwise I may forget >> just like I forgot to remove "1.5" from the stats page before releasing >> 1.7.1. Attached. Marcin --- doc/configuration.txt.old 2016-12-12 22:57:04.0 +0100 +++ doc/configuration.txt 2016-12-13 12:34:47.385694584 +0100 @@ -13182,7 +13182,7 @@ fe_id : integer Returns an integer containing the current frontend's id. It can be used in - backends to check from which backend it was called, or to stick all users + backends to check from which frontend it was called, or to stick all users coming via a same frontend to the same server. fe_name : string
Re: [External] Re: [PATCH] Add fe_name/be_name fetchers next to existing fe_id/be_id
On 12/12/2016 03:14 PM, Willy Tarreau wrote: > Just merged now, thanks. A few weeks ago someone needed it, I guess it > was for logging, and I thought we had it though it was not the case, so > I realized we were missing it. In fact we still have quite a number of > fields available as log tags which do not have an equivalent sample fetch > function. It would be nice to ensure we have all of them and document the > equivalence as well in the logformat table. Thank you Willy. For logging you can use %f (which is already there) although in some places fetcher is required and then you are stuck. BTW: Once I can "extract" frontend name is there a way to compare "dynamic" data (eg. frontend name) with another "dynamic" data (eg. host header) ? From what I can see we can easily compare "dynamic" data with "static" data (eg. some predefined string), but not eg. 2 variables. Regards, Marcin Side note: In docs/configuration.txt I came across this: fe_id : integer Returns an integer containing the current frontend's id. It can be used in backends to check from which backend it was called, or to stick all users coming via a same frontend to the same server. shouldn't this contain: ...It can be used in backends to check from which FRONTEND it was called... ?
[PATCH] Add fe_name/be_name fetchers next to existing fe_id/be_id
Hi, These 2 patches add ability to fetch frontend/backend name in your logic, so they can be used later to make routing decisions (fe_name) or taking some actions based on backend which responded to request (be_name). In our case we needed a fetcher to be able to extract information we needed from frontend name. Regards, Marcin Deranek --- haproxy-1.7.0/src/backend.c.old 2016-11-25 16:39:17.0 +0100 +++ haproxy-1.7.0/src/backend.c 2016-12-12 12:57:24.683678340 +0100 @@ -1679,6 +1679,24 @@ return 1; } +/* set string to the name of the backend */ +static int +smp_fetch_be_name(const struct arg *args, struct sample *smp, const char *kw, void *private) +{ + if (!smp->strm) + return 0; + + smp->data.u.str.str = (char *)smp->strm->be->id; + if (!smp->data.u.str.str) + return 0; + + smp->data.type = SMP_T_STR; + smp->flags = SMP_F_CONST; + smp->data.u.str.len = strlen(smp->data.u.str.str); + + return 1; +} + /* set temp integer to the id of the server */ static int smp_fetch_srv_id(const struct arg *args, struct sample *smp, const char *kw, void *private) @@ -1801,6 +1819,7 @@ { "avg_queue", smp_fetch_avg_queue_size, ARG1(1,BE), NULL, SMP_T_SINT, SMP_USE_INTRN, }, { "be_conn", smp_fetch_be_conn,ARG1(1,BE), NULL, SMP_T_SINT, SMP_USE_INTRN, }, { "be_id", smp_fetch_be_id, 0, NULL, SMP_T_SINT, SMP_USE_BKEND, }, + { "be_name", smp_fetch_be_name,0, NULL, SMP_T_STR, SMP_USE_BKEND, }, { "be_sess_rate", smp_fetch_be_sess_rate, ARG1(1,BE), NULL, SMP_T_SINT, SMP_USE_INTRN, }, { "connslots", smp_fetch_connslots, ARG1(1,BE), NULL, SMP_T_SINT, SMP_USE_INTRN, }, { "nbsrv", smp_fetch_nbsrv, ARG1(1,BE), NULL, SMP_T_SINT, SMP_USE_INTRN, }, --- haproxy-1.7.0/doc/configuration.txt.old 2016-11-25 16:39:17.0 +0100 +++ haproxy-1.7.0/doc/configuration.txt 2016-12-12 13:35:32.542340475 +0100 @@ -13072,6 +13072,10 @@ Returns an integer containing the current backend's id. It can be used in frontends with responses to check which backend processed the request. +be_name : string + Returns a string containing the current backend's name. It can be used in + frontends with responses to check which backend processed the request. + dst : ip This is the destination IPv4 address of the connection on the client side, which is the address the client connected to. It can be useful when running --- haproxy-1.7.0/src/frontend.c.old 2016-11-25 16:39:17.0 +0100 +++ haproxy-1.7.0/src/frontend.c 2016-12-12 12:44:05.573873525 +0100 @@ -167,6 +167,20 @@ return 1; } +/* set string to the name of the frontend */ +static int +smp_fetch_fe_name(const struct arg *args, struct sample *smp, const char *kw, void *private) +{ + smp->data.u.str.str = (char *)smp->sess->fe->id; + if (!smp->data.u.str.str) + return 0; + + smp->data.type = SMP_T_STR; + smp->flags = SMP_F_CONST; + smp->data.u.str.len = strlen(smp->data.u.str.str); + return 1; +} + /* set temp integer to the number of HTTP requests per second reaching the frontend. * Accepts exactly 1 argument. Argument is a frontend, other types will cause * an undefined behaviour. @@ -213,6 +227,7 @@ static struct sample_fetch_kw_list smp_kws = {ILH, { { "fe_conn", smp_fetch_fe_conn, ARG1(1,FE), NULL, SMP_T_SINT, SMP_USE_INTRN, }, { "fe_id",smp_fetch_fe_id,0, NULL, SMP_T_SINT, SMP_USE_FTEND, }, + { "fe_name", smp_fetch_fe_name, 0, NULL, SMP_T_STR, SMP_USE_FTEND, }, { "fe_req_rate", smp_fetch_fe_req_rate, ARG1(1,FE), NULL, SMP_T_SINT, SMP_USE_INTRN, }, { "fe_sess_rate", smp_fetch_fe_sess_rate, ARG1(1,FE), NULL, SMP_T_SINT, SMP_USE_INTRN, }, { /* END */ }, --- haproxy-1.7.0/doc/configuration.txt.old 2016-11-25 16:39:17.0 +0100 +++ haproxy-1.7.0/doc/configuration.txt 2016-12-12 13:38:05.235730351 +0100 @@ -13164,6 +13168,11 @@ backends to check from which backend it was called, or to stick all users coming via a same frontend to the same server. +fe_name : string + Returns a string containing the current frontend's name. It can be used in + backends to check from which frontend it was called, or to stick all users + coming via a same frontend to the same server. + sc_bytes_in_rate([,]) : integer sc0_bytes_in_rate([]) : integer sc1_bytes_in_rate([]) : integer
Re: Haproxy SSL performance vs nginx
On Tue, 4 Oct 2016 14:45:16 +0530 Rajesh Mahajan wrote: > Please find attached new test result using httpress tool. > Configuration is remain same for both nginx and haproxy shared > earlier. > > Summary Report > *Haproxy:* > > TOTALS: 5000 connect, 5000 requests, 5000 success, 0 fail, 500 (500) > real concurrency > TRAFFIC: 10 avg bytes, 230 avg overhead, 5 bytes, 115 overhead > TIMING: 25.253 seconds, 197 rps, 46 kbps, 2525.3 ms avg req time > > *Nginx:* > > TOTALS: 5000 connect, 5000 requests, 5000 success, 0 fail, 500 (500) > real concurrency > TRAFFIC: 10 avg bytes, 230 avg overhead, 5 bytes, 115 overhead > TIMING: 6.439 seconds, 776 rps, 181 kbps, 643.9 ms avg req time Are you sure you are comparing apples with apples ? As far as I can see in both cases you are requesting /index.html. In case of Nginx this is served locally by Nginx. In case of HAProxy on the other hand this goes the the backend server which means HAProxy acts as a proxy which means it has to do more work / takes more time. Cheers, Marcin
Re: Haproxy SSL performance vs nginx
On Tue, 4 Oct 2016 14:24:13 +0530 Rajesh Mahajan wrote: > Please check the ssl_haproxy.cfg .We have defined max value as below > > global > maxconn 2 > maxconnrate 15000 This is a different setting. This is per-process. By default frontend has 2000 connection limit. http://cbonte.github.io/haproxy-dconv/1.6/configuration.html#maxconn%20(Performance%20tuning) vs http://cbonte.github.io/haproxy-dconv/1.6/configuration.html#maxconn%20(Alphabetically%20sorted%20keywords%20reference) Marcin
Re: Haproxy SSL performance vs nginx
Hi, On Tue, 4 Oct 2016 10:55:08 +0530 Rajesh Mahajan wrote: > Please find attached configuration files for both nginx and haproxy. > Could you please share your results wrt to nginx and tell me which > http benchmark tool you are using for testing. From what I see you did not specify maxconn limit in defaults nor frontend sections, so effectively you will be limited to 2000 concurrent connections on frontend (we run into this problem during our tests). Due to the fact that we needed to test our setup up to 10Gb/s we had to build our own tools as currently available scaled "only" up to ~2-3Gb/s. We used multi-threaded tools like wrk2, httpress (with some patches) and weighttp as a starting point and build scripts around them to aggregate data from multiple machines running those. I started to prefer httpress (with some patches) as it allows to test keepalive/non-keepalive connections (as opposite to wrk2). On the other hand wrk2 has some nice Lua scripting capabilities which might be useful in certain scenarios like replaying traffic. Unfortunately right now I cannot find any "trustworthy" results we have obtained while comparing HAProxy and nginx. During our testing we discovered that Nginx does not scale linearly when increasing amount of workers - it was even worse: increasing amount of workers degraded performance. Later this problem was mitigated, but not completely resolved resulting up to 20% performance degradation with 24 cores/workers running. Proper fix would require significant architectural changes of Nginx, so we were left only with a workaround. Overall HAProxy and nginx were comparable (HAProxy still faster), but due to above bug nginx was visibly behind when number of workers increased. When we decided to use HAProxy our focus moved towards tuning HAProxy rather than comparing HAProxy and nginx. We have some data on our Wiki from time when we compared HAProxy and nginx, but setup and performance isn't nowhere near what we currently use/get (Those benchmarks did not use our distributed benchmark infrastructure to obtain results, so effectively they were limited to 1Gb/s of traffic due to network interface throughput). Regards, Marcin
Re: Haproxy SSL performance vs nginx
Hi, On Fri, 30 Sep 2016 09:09:07 +0530 Rajesh Mahajan wrote: > NGINX CONFIG > > server { > > listen 8444; > listen 8443 ssl; What are your other nginx settings (eg. amount of workers / amount of worker connections etc.) ? During out tests we had completely different results: HAProxy was faster than nginx. Regards, Marcin Deranek