Re: [PATCH] enable arm64 builds in travis-ci

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

пт, 8 мая 2020 г. в 11:35, Martin Grigorov :

> Hi,
>
> I think I understand why it started failing.
> It must have started failing when you updated the version of OpenSSL.
> .travis.yml caches ~/opt folder between builds. After the update to 1.1.1f
> the build doesn't see the OpenSSL binaries in the cache anymore and tries
> to download it and build it.
> But as I've noticed in my attempt to build HAProxy with Docker+QEMU the
> build of OpenSSL is taking too long.
> The build of OpenSSL is wrapped with travis_wait to reduce the writes to
> stdout but the default time for travis_wait is 20 mins and this is not
> enough to build OpenSSL.
> Due to
> https://travis-ci.community/t/output-is-truncated-heavily-in-arm64-when-a-command-hangs/7630
>  TravisCI
> does not properly report that the problem is at build_ssl() step but shows
> the last chunk of the buffered response and this confuses us all.
> I think the build will become green if we extend travis_wait to a higher
> value (
> https://docs.travis-ci.com/user/common-build-problems/#build-times-out-because-no-output-was-received).
> I don't remember where I have read it but I think the upper limit is 120
> mins.
> @Willy: could you please change
> https://github.com/haproxy/haproxy/blob/master/.travis.yml#L112 to:
>
> travis_wait 120 bash -c 'scripts/build-ssl.sh >build-ssl.log 2>&1' || (cat
> build-ssl.log && exit 1)
>
> i.e. add '120' after travis_wait
>


https://travis-ci.com/github/chipitsine/haproxy/jobs/329528056 - as you can
see full openssl build took 640s
it is longer than 10 minutes


>
> This should give it the time to download and install OpenSSL 1.1.1f and to
> cache it. If the build passes once then the next builds should be much
> faster because OpenSSL will be used from the cache.
>
> Regards,
> Martin
>
> On Fri, May 8, 2020 at 9:18 AM Willy Tarreau  wrote:
>
>> Hi Martin,
>>
>> On Fri, May 08, 2020 at 08:56:07AM +0300, Martin Grigorov wrote:
>> > Unfortunately it is not good:
>> > https://travis-ci.com/github/haproxy/haproxy/jobs/329657180
>>
>> Indeed it's still not fixed on Travis' side. However what Ilya did
>> actually worked, in that the status is not reported as a global
>> build failure anymore. This allows us to continue to monitor if
>> and when this issue finally resolves on the build infrastructure.
>> It's also possible that they're not aware of the problem due to
>> too few people using arm64. If someone has contacts there it might
>> be worth checking with them. All we know for now is that it seems
>> to stop moving while setting up libpcre2. Maybe there's a bug in
>> a script in this package, resulting in a prompt for a question
>> which never gets a response :-/  But that's something we can't
>> check since we don't have access to an interactive shell there
>> to diagnose.
>>
>> Willy
>>
>

Re: [PATCH] enable arm64 builds in travis-ci

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

пт, 8 мая 2020 г. в 11:27, Willy Tarreau :

> On Fri, May 08, 2020 at 11:12:00AM +0500,  ??? wrote:
> > btw, in my fork it is green
> > https://travis-ci.com/github/chipitsine/haproxy/jobs/329528056
>
> Cool, that tends to confirm the issue remains a bit random. There
> is actually something I don't like in the build report, which is
> that the "Setting up libpcre2" line is truncated. This definitely
> indicates that output is not line-buffered and that could mean that
> it in fact fails multiple lines later (processing triggers or any
> such thing) but that the successful operations were not properly
> logged. At this point I guess it's the apt-get utility which does
> not properly configure its stdout, but it could also be anything
> in between. In any case I doubt we'll have a big handle on this.
>

there are few options

1) setup separate "apt" key for "arm64", we do not use pcre2 here

2) use direct "apt-get" call, not travis "apt" plugin (maybe we can control
buffering that way)

3) interoperate with travis via support channel

4) wait until it is resolved by itself

Martin ?



>
> Willy
>

Re: [PATCH] enable arm64 builds in travis-ci

[Martin Grigorov]

Hi,

I think I understand why it started failing.
It must have started failing when you updated the version of OpenSSL.
.travis.yml caches ~/opt folder between builds. After the update to 1.1.1f
the build doesn't see the OpenSSL binaries in the cache anymore and tries
to download it and build it.
But as I've noticed in my attempt to build HAProxy with Docker+QEMU the
build of OpenSSL is taking too long.
The build of OpenSSL is wrapped with travis_wait to reduce the writes to
stdout but the default time for travis_wait is 20 mins and this is not
enough to build OpenSSL.
Due to
https://travis-ci.community/t/output-is-truncated-heavily-in-arm64-when-a-command-hangs/7630
TravisCI
does not properly report that the problem is at build_ssl() step but shows
the last chunk of the buffered response and this confuses us all.
I think the build will become green if we extend travis_wait to a higher
value (
https://docs.travis-ci.com/user/common-build-problems/#build-times-out-because-no-output-was-received).
I don't remember where I have read it but I think the upper limit is 120
mins.
@Willy: could you please change
https://github.com/haproxy/haproxy/blob/master/.travis.yml#L112 to:

travis_wait 120 bash -c 'scripts/build-ssl.sh >build-ssl.log 2>&1' || (cat
build-ssl.log && exit 1)

i.e. add '120' after travis_wait

This should give it the time to download and install OpenSSL 1.1.1f and to
cache it. If the build passes once then the next builds should be much
faster because OpenSSL will be used from the cache.

Regards,
Martin

On Fri, May 8, 2020 at 9:18 AM Willy Tarreau  wrote:

> Hi Martin,
>
> On Fri, May 08, 2020 at 08:56:07AM +0300, Martin Grigorov wrote:
> > Unfortunately it is not good:
> > https://travis-ci.com/github/haproxy/haproxy/jobs/329657180
>
> Indeed it's still not fixed on Travis' side. However what Ilya did
> actually worked, in that the status is not reported as a global
> build failure anymore. This allows us to continue to monitor if
> and when this issue finally resolves on the build infrastructure.
> It's also possible that they're not aware of the problem due to
> too few people using arm64. If someone has contacts there it might
> be worth checking with them. All we know for now is that it seems
> to stop moving while setting up libpcre2. Maybe there's a bug in
> a script in this package, resulting in a prompt for a question
> which never gets a response :-/  But that's something we can't
> check since we don't have access to an interactive shell there
> to diagnose.
>
> Willy
>

Re: [PATCH] enable arm64 builds in travis-ci

[Willy Tarreau]

On Fri, May 08, 2020 at 11:12:00AM +0500,  ??? wrote:
> btw, in my fork it is green
> https://travis-ci.com/github/chipitsine/haproxy/jobs/329528056

Cool, that tends to confirm the issue remains a bit random. There
is actually something I don't like in the build report, which is
that the "Setting up libpcre2" line is truncated. This definitely
indicates that output is not line-buffered and that could mean that
it in fact fails multiple lines later (processing triggers or any
such thing) but that the successful operations were not properly
logged. At this point I guess it's the apt-get utility which does
not properly configure its stdout, but it could also be anything
in between. In any case I doubt we'll have a big handle on this.

Willy

Re: [PATCH] enable arm64 builds in travis-ci

[Willy Tarreau]

Hi Martin,

On Fri, May 08, 2020 at 08:56:07AM +0300, Martin Grigorov wrote:
> Unfortunately it is not good:
> https://travis-ci.com/github/haproxy/haproxy/jobs/329657180

Indeed it's still not fixed on Travis' side. However what Ilya did
actually worked, in that the status is not reported as a global
build failure anymore. This allows us to continue to monitor if
and when this issue finally resolves on the build infrastructure.
It's also possible that they're not aware of the problem due to
too few people using arm64. If someone has contacts there it might
be worth checking with them. All we know for now is that it seems
to stop moving while setting up libpcre2. Maybe there's a bug in
a script in this package, resulting in a prompt for a question
which never gets a response :-/  But that's something we can't
check since we don't have access to an interactive shell there
to diagnose.

Willy

Re: [PATCH] enable arm64 builds in travis-ci

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

пт, 8 мая 2020 г. в 10:56, Martin Grigorov :

> Hi all,
>
> On Thu, May 7, 2020 at 11:56 PM Willy Tarreau  wrote:
>
>> Hi Ilya,
>>
>> On Thu, May 07, 2020 at 09:19:48PM +0500,  ??? wrote:
>> > Hello,
>> >
>> > let us enable arm64 builds back.
>>
>> Good idea, just merged now. Let's see how that ends up now.
>>
>
> Unfortunately it is not good:
> https://travis-ci.com/github/haproxy/haproxy/jobs/329657180
>

that's why it is marked as "allow to fail"


btw, in my fork it is green
https://travis-ci.com/github/chipitsine/haproxy/jobs/329528056



>
> Martin
>
>
>>
>> Thanks,
>> Willy
>>
>>

Re: [PATCH] enable arm64 builds in travis-ci

[Martin Grigorov]

Hi all,

On Thu, May 7, 2020 at 11:56 PM Willy Tarreau  wrote:

> Hi Ilya,
>
> On Thu, May 07, 2020 at 09:19:48PM +0500,  ??? wrote:
> > Hello,
> >
> > let us enable arm64 builds back.
>
> Good idea, just merged now. Let's see how that ends up now.
>

Unfortunately it is not good:
https://travis-ci.com/github/haproxy/haproxy/jobs/329657180

Martin


>
> Thanks,
> Willy
>
>

Re: [PATCH] enable arm64 builds in travis-ci

[Willy Tarreau]

Hi Ilya,

On Thu, May 07, 2020 at 09:19:48PM +0500,  ??? wrote:
> Hello,
> 
> let us enable arm64 builds back.

Good idea, just merged now. Let's see how that ends up now.

Thanks,
Willy

Re: [PATCH] MINOR: crypto: Add digest and hmac converters

[Willy Tarreau]

Patrick,

On Thu, May 07, 2020 at 07:42:49PM +0200, Tim Düsterhus wrote:
> >> Please note that I review patches on a voluntary basis. I'm not an
> >> "employed first level reviewer".
> > 
> > That's not what I meant. I thought that when regular participants on
> > this list do not spot the errors of first time contributes, it can't be
> > that obvious and directing to CONTRIBUTING might not be enough.
> 
> I agree with you that Willy's reply was overly blunt in this case.
> Especially since you demonstrated an effort to take my review into a
> account and he noticed my Ack (implying it can't be too bad).

In case that happened, I really didn't mean to offend you. It's just
that I'm investing a huge amount of time doing something that nobody
likes, to be honest, which is documenting processes to help everyone
be more autonomous and let the project scale better and rely less on
my shoulders. And certain days I spend 100% of my time dealing with
issues which should never have happened in the first place if the doc
had been read, making me wonder if it's really worth wasting time
documenting. So these days when I end up on yet-another-one in that
band, it can be one too much and I let it rot until next time I have
the opportunity to visit it again.

This is also the reason why I ask people to post their patches here (as
you did), so that comments are shared and help others get better when
it's their turn. No review here is meant to judge the person or attack
anyone. We can criticize code and processes but not anyone's work. The
first and unrivaled bug author here is me, so everyone is safe regarding
this, nobody risks to be laughed at!

I'm always grateful for a contribution because I know how painful it is
to contribute to a project you're not accustomed to, and I really value
the effort way more than the result (which is why I continue to silently
fix some contributions instead of asking people to resubmit with a minor
change).

The root cause of the problem that happened is not trivial, but it's not
complicated to understand either once you get the whole picture. Today
the development time is spent this way:

1: analyzing issues and fixing bugs (*more* than 50% of the time)
2: reviewing code to try to reduce future impact on #1 above
3: trying to untangle the mess of old code when trying to add something
4: actually adding the 5 lines that were really necessary for a new
   given feature

#1 is mostly affected by "how is it possible to do end up in this situation?",
immediately followed by "but WHY does it HAVE TO work like this?". This "WHY"
is exactly what must appear in a commit message, it's the *justification*
that a change is necessary. Consider that you're *selling* your patch to the
community and that the people maintaining the project are going to devote a
little bit more of their own time to keep your code alive forever. You want
to convince them that your change is really worth the invested time. And
sadly this "why" is often missing in a lot of patches. It does have
devastating effects on bug fixing. It happens sometimes that an issue is
incorrectly fixed because the original intent of a change was not well
understood, and that it's only after 5-6 iterative fixes, and in-person
discussions with several engineers that it becomes obvious that everyone
went down the wrong road and that this has to be completely unrolled and
done differently. A good example of this is all the mess that finally
ended with this commit:

  04400bc787 ("BUG/MAJOR: stream-int: Don't receive data from mux until 
SI_ST_EST is reached")

This is extremely time-consuming and the cause of many of the bugs that
take a long time to fix.

Further, when backporting fixes, there are always differences between
branches. We try to limit the amount of annoying changes that further add
to the maintenance cost, because seeing patches fail to apply or build in
each and every stable branch, or requiring complex adaptations is a real
pain and is the *only* cause for stable branches sometimes lagging behind
for a while and missing some fixes. As a hint, if your patch has zero
impact on any existing setup and can trivially be backported, just asking
kindly will often be enough to get it backported. Some here already know
this and use this. This is important as it saves some users from the risk
of upgrading to a much more recent branch for something trivial. It often
happens for sample fetch functions or converters.

Now you might think "but my change is trivial and will not cause any
issue". Before responding to this, let me show you what I spent my whole
afternoon on because I think it's very representative of this process.

In 2018 a user reported a bug on Discourse. Lukas helped the person,
diagnosed the issue and proposed this fix:

  commit fd9b68c48ecdba5e7971899f4eec315c8e3a3cfe
  Author: Lukas Tribus 
  Date:   Sat Oct 27 20:06:59 2018 +0200

BUG/MINOR: only mark connections private if NTLM is detected

 

Re: [PATCH] MINOR: crypto: Add digest and hmac converters

[Tim Düsterhus]

Patrick,

Am 07.05.20 um 18:54 schrieb Patrick Gansterer:
>> I skipped the patch (that's why I did not ACK the first, but only the
>> second one), because it was a very large change of code that was just
>> moved. Nonetheless I should have noticed the missing body and noted that
>> during my review.
> 
> IMHO, that's a patch you usually do not want review.

Yes, that's why I did not. However the commit message is not directly
part of the patch, so I should have.

>> Please note that I review patches on a voluntary basis. I'm not an
>> "employed first level reviewer".
> 
> That's not what I meant. I thought that when regular participants on
> this list do not spot the errors of first time contributes, it can't be
> that obvious and directing to CONTRIBUTING might not be enough.

I agree with you that Willy's reply was overly blunt in this case.
Especially since you demonstrated an effort to take my review into a
account and he noticed my Ack (implying it can't be too bad).

>> Liking HAProxy and wanting to give something back is my motivation as
>> well. I am very sorry to see how this experience went for you. If it is
>> of any help to you: This is definitely not how it usually goes.
> 
> Then here is my next try. ;-)
> 
> I've rebased my changes to reflect the recent changes and added the
> missing description to the first patch.

I've now taken a look at both patches now and both are:

Reviewed-by: Tim Duesterhus 

Best regards
Tim Düsterhus

I,m interesting guest post..https://www.haproxy.org/

[aliexa bless]

   I,m interesting guest post..
can you published my article with link on your site
https://www.haproxy.org/
we need Dofollow link ..

we need Permanent post

Payment method PayPal

please reply me as soon as possible
thanks..

Re: [PATCH] MINOR: crypto: Add digest and hmac converters

[Patrick Gansterer]

On 07.05.20 18:23, Tim Düsterhus wrote:> This is about your "[PATCH 1/2] 
MINOR: crypto: Move aes_gcm_dec

implementation into new file". It does not contain a body, but instead
just a subject.


Ok, agree.


I skipped the patch (that's why I did not ACK the first, but only the
second one), because it was a very large change of code that was just
moved. Nonetheless I should have noticed the missing body and noted that
during my review.


IMHO, that's a patch you usually do not want review.


Please note that I review patches on a voluntary basis. I'm not an
"employed first level reviewer".


That's not what I meant. I thought that when regular participants on 
this list do not spot the errors of first time contributes, it can't be 
that obvious and directing to CONTRIBUTING might not be enough.



Liking HAProxy and wanting to give something back is my motivation as
well. I am very sorry to see how this experience went for you. If it is
of any help to you: This is definitely not how it usually goes.


Then here is my next try. ;-)

I've rebased my changes to reflect the recent changes and added the 
missing description to the first patch.


- Patrick
>From 9125c6df81e135275c450a0be32bcd0b58ef2099 Mon Sep 17 00:00:00 2001
From: Patrick Gansterer 
Date: Sun, 17 Jun 2018 11:21:11 +0200
Subject: [PATCH 1/2] MINOR: crypto: Move aes_gcm_dec implementation into new
 file

aes_gcm_dec is independent of the TLS implementation and fits better
in a separate file dedicated to crypto functionality.
This gives converters which depend on OpenSSL a clear home.
---
 Makefile   |   2 +-
 src/crypto.c   | 163 +
 src/ssl_sock.c | 142 --
 3 files changed, 164 insertions(+), 143 deletions(-)
 create mode 100644 src/crypto.c

diff --git a/Makefile b/Makefile
index 1e4213989..2dea46368 100644
--- a/Makefile
+++ b/Makefile
@@ -542,7 +542,7 @@ OPTIONS_LDFLAGS += $(if $(SSL_LIB),-L$(SSL_LIB)) -lssl -lcrypto
 ifneq ($(USE_DL),)
 OPTIONS_LDFLAGS += -ldl
 endif
-OPTIONS_OBJS  += src/ssl_sock.o
+OPTIONS_OBJS  += src/crypto.o src/ssl_sock.o
 endif
 
 # The private cache option affect the way the shctx is built
diff --git a/src/crypto.c b/src/crypto.c
new file mode 100644
index 0..74b92eee5
--- /dev/null
+++ b/src/crypto.c
@@ -0,0 +1,163 @@
+/*
+ * Crypto converters
+ *
+ * Copyright 2020 Nenad Merdanovic 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ */
+
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#include 
+
+#if (HA_OPENSSL_VERSION_NUMBER >= 0x1000100fL)
+static inline int sample_conv_var2smp_str(const struct arg *arg, struct sample *smp)
+{
+	switch (arg->type) {
+	case ARGT_STR:
+		smp->data.type = SMP_T_STR;
+		smp->data.u.str = arg->data.str;
+		return 1;
+	case ARGT_VAR:
+		if (!vars_get_by_desc(&arg->data.var, smp))
+return 0;
+		if (!sample_casts[smp->data.type][SMP_T_STR])
+return 0;
+		if (!sample_casts[smp->data.type][SMP_T_STR](smp))
+return 0;
+		return 1;
+	default:
+		return 0;
+	}
+}
+
+static int check_aes_gcm(struct arg *args, struct sample_conv *conv,
+		  const char *file, int line, char **err)
+{
+	switch(args[0].data.sint) {
+	case 128:
+	case 192:
+	case 256:
+		break;
+	default:
+		memprintf(err, "key size must be 128, 192 or 256 (bits).");
+		return 0;
+	}
+	/* Try to decode a variable. */
+	vars_check_arg(&args[1], NULL);
+	vars_check_arg(&args[2], NULL);
+	vars_check_arg(&args[3], NULL);
+	return 1;
+}
+
+/* Arguments: AES size in bits, nonce, key, tag. The last three arguments are base64 encoded */
+static int sample_conv_aes_gcm_dec(const struct arg *arg_p, struct sample *smp, void *private)
+{
+	struct sample nonce, key, aead_tag;
+	struct buffer *smp_trash, *smp_trash_alloc;
+	EVP_CIPHER_CTX *ctx;
+	int dec_size, ret;
+
+	smp_set_owner(&nonce, smp->px, smp->sess, smp->strm, smp->opt);
+	if (!sample_conv_var2smp_str(&arg_p[1], &nonce))
+		return 0;
+
+	smp_set_owner(&key, smp->px, smp->sess, smp->strm, smp->opt);
+	if (!sample_conv_var2smp_str(&arg_p[2], &key))
+		return 0;
+
+	smp_set_owner(&aead_tag, smp->px, smp->sess, smp->strm, smp->opt);
+	if (!sample_conv_var2smp_str(&arg_p[3], &aead_tag))
+		return 0;
+
+	smp_trash = get_trash_chunk();
+	smp_trash_alloc = alloc_trash_chunk();
+	if (!smp_trash_alloc)
+		return 0;
+
+	ctx = EVP_CIPHER_CTX_new();
+
+	if (!ctx)
+		goto err;
+
+	dec_size = base64dec(nonce.data.u.str.area, nonce.data.u.str.data, smp_trash->area, smp_trash->size);
+	if (dec_size < 0)
+		goto err;
+	smp_trash->data = dec_size;
+
+	/* Set cipher type and mode */
+	switch(arg_p[0].data.sint) {
+	case 128:
+		EVP_DecryptInit_ex(ctx, EVP_aes_128_gcm(), NULL, NULL, NULL);
+		break;
+	case 192:
+		EVP_DecryptInit_ex(ctx, EVP_aes_192_gcm(),

Re: [PATCH] MINOR: crypto: Add digest and hmac converters

[Tim Düsterhus]

Patrick,

Am 07.05.20 um 18:04 schrieb Patrick Gansterer:
> On 07.05.20 17:35, Willy Tarreau wrote:
>> Indeed. I encourage to ping again after one week because usually when you
>> restart with a new week of work, the previous one is definitely in old
>> history and will only be revisited by pure luck.
> 
> I don't want to look impatient, so I waited 2 weeks. ;-)
> 
>> With this said, I remember having noticed Tim's ack and was about to take
>> the series until I noticed there was a first patch with no commit message
>> to justify the change and postponed the reply because I had other things
>> to do than to rehash what's already in CONTRIBUTING again and again :-/
> 
> I'm not sure how I should read this. I wrote an explanation into the
> commit message and tried to match already existing messages. If I look
> at commits in the last days like 0e9d87bf06 or de80201460 there are no
> very long commit messages either.
> 
> If I should write an essay about my use case into the commit message I
> could do that, but that's something a "first level reviewer" could
> demand already.

This is about your "[PATCH 1/2] MINOR: crypto: Move aes_gcm_dec
implementation into new file". It does not contain a body, but instead
just a subject.

I skipped the patch (that's why I did not ACK the first, but only the
second one), because it was a very large change of code that was just
moved. Nonetheless I should have noticed the missing body and noted that
during my review.

Please note that I review patches on a voluntary basis. I'm not an
"employed first level reviewer".

> I read the CONTRIBUTING text before submitting my patch, since I don't
> like to repeat myself also, but it's hard to get everything right in the
> first patch. Sorry for that.

The "missing body" Willy was referring to is here, if you'd like to read
up on it:
https://github.com/haproxy/haproxy/blob/f82ea4ae4ca4a6fca70a6e874643db887a39f037/CONTRIBUTING#L562-L567

> I really like haproxy and want to give something back, but I'm not sure
> if I want to do that in the future with the experience I had so far. :-(
> 

Liking HAProxy and wanting to give something back is my motivation as
well. I am very sorry to see how this experience went for you. If it is
of any help to you: This is definitely not how it usually goes.

Best regards
Tim Düsterhus

[PATCH] enable arm64 builds in travis-ci

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

Hello,

let us enable arm64 builds back.


Cheers,
Ilya Shipitcin
From 8c68b4494ba37469de86798fd7c492e38a2a8668 Mon Sep 17 00:00:00 2001
From: Ilya Shipitsin 
Date: Thu, 7 May 2020 20:57:26 +0500
Subject: [PATCH] CI: travis-ci: enable arm64 builds again

travis-ci arm64 were temporarily disabled due to travis instability.
let us enable them in "allowed to fail" mode.
---
 .travis.yml | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 345c08afe..0d7268791 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -43,13 +43,11 @@ matrix:
 if: type == push
 compiler: clang
 env: TARGET=linux-glibc OPENSSL_VERSION=1.1.1f CC=clang-9
-##
-## temporarily disabled, until arm64 runners become stable
-#  - os: linux
-#arch: arm64
-#if: type == push
-#compiler: clang
-#env: TARGET=linux-glibc OPENSSL_VERSION=1.1.1f CC=clang-9
+  - os: linux
+arch: arm64
+if: type == push
+compiler: clang
+env: TARGET=linux-glibc OPENSSL_VERSION=1.1.1f CC=clang-9
   - os: linux
 arch: s390x
 if: type == push
@@ -101,6 +99,11 @@ matrix:
 before_script:
   - git clone http://git.1wt.eu/git/libslz.git/
   - cd libslz && make && make PREFIX=${HOME}/opt install && cd ..
+  allow_failures:
+  - os: linux
+arch: arm64
+if: type == push
+compiler: clang
 
 install:
   - git clone https://github.com/VTest/VTest.git ../vtest
-- 
2.25.4

Re: [PATCH] MINOR: crypto: Add digest and hmac converters

[Patrick Gansterer]

On 07.05.20 17:35, Willy Tarreau wrote:

Indeed. I encourage to ping again after one week because usually when you
restart with a new week of work, the previous one is definitely in old
history and will only be revisited by pure luck.


I don't want to look impatient, so I waited 2 weeks. ;-)


With this said, I remember having noticed Tim's ack and was about to take
the series until I noticed there was a first patch with no commit message
to justify the change and postponed the reply because I had other things
to do than to rehash what's already in CONTRIBUTING again and again :-/


I'm not sure how I should read this. I wrote an explanation into the 
commit message and tried to match already existing messages. If I look 
at commits in the last days like 0e9d87bf06 or de80201460 there are no 
very long commit messages either.


If I should write an essay about my use case into the commit message I 
could do that, but that's something a "first level reviewer" could 
demand already.


I read the CONTRIBUTING text before submitting my patch, since I don't 
like to repeat myself also, but it's hard to get everything right in the 
first patch. Sorry for that.


I really like haproxy and want to give something back, but I'm not sure 
if I want to do that in the future with the experience I had so far. :-(


- Patrick

Re: [PATCH] MINOR: crypto: Add digest and hmac converters

[Willy Tarreau]

On Thu, May 07, 2020 at 04:48:50PM +0200, Christopher Faulet wrote:
> Le 07/05/2020 à 13:03, Patrick Gansterer a écrit :
> > Hi,
> > 
> > On 22.04.20 18:30, Tim Düsterhus wrote:
> > > I don't find anything to complain about now. I'll now leave it up to the
> > > authority to either apply or complain.
> > 
> > How long does it usually take to get a response?
> > 
> > I posted a similar patch already 2 years ago and never got a response
> > from the maintainers. It would be nice if it could make its way into
> > 2.2, since it's not invasive and has proper testing.
> > If gets rejected I'm ok too, but it would be nice to know at least.
> > 
> > - Patrick
> > 
> 
> Hi Patrick,
> 
> Really sorry. 2 years! You're really patient :) As Tim said, don't hesitate
> to kindly harass us in this situation. It is pretty easy to miss some
> patches, lost in all other mails.

Indeed. I encourage to ping again after one week because usually when you
restart with a new week of work, the previous one is definitely in old
history and will only be revisited by pure luck.

With this said, I remember having noticed Tim's ack and was about to take
the series until I noticed there was a first patch with no commit message
to justify the change and postponed the reply because I had other things
to do than to rehash what's already in CONTRIBUTING again and again :-/

Cheers,
Willy

Re: [PATCH] MINOR: crypto: Add digest and hmac converters

[Christopher Faulet]

Le 07/05/2020 à 16:48, Christopher Faulet a écrit :

Le 07/05/2020 à 13:03, Patrick Gansterer a écrit :

Hi,

On 22.04.20 18:30, Tim Düsterhus wrote:

I don't find anything to complain about now. I'll now leave it up to the
authority to either apply or complain.


How long does it usually take to get a response?

I posted a similar patch already 2 years ago and never got a response
from the maintainers. It would be nice if it could make its way into
2.2, since it's not invasive and has proper testing.
If gets rejected I'm ok too, but it would be nice to know at least.

- Patrick



Hi Patrick,

Really sorry. 2 years! You're really patient :) As Tim said, don't hesitate to
kindly harass us in this situation. It is pretty easy to miss some patches, lost
in all other mails.

I will review your patches. At first glance, it looks good for me. I will just
enclose sample_conv_var2smp_str in the #if/#endif to avoid warning with older
openssl versions.



Sorry I misread your patch, it is ok for sample_conv_var2smp_str function...

--
Christopher Faulet

Re: [PATCH] MINOR: crypto: Add digest and hmac converters

[Christopher Faulet]

Le 07/05/2020 à 13:03, Patrick Gansterer a écrit :

Hi,

On 22.04.20 18:30, Tim Düsterhus wrote:

I don't find anything to complain about now. I'll now leave it up to the
authority to either apply or complain.


How long does it usually take to get a response?

I posted a similar patch already 2 years ago and never got a response
from the maintainers. It would be nice if it could make its way into
2.2, since it's not invasive and has proper testing.
If gets rejected I'm ok too, but it would be nice to know at least.

- Patrick



Hi Patrick,

Really sorry. 2 years! You're really patient :) As Tim said, don't hesitate to 
kindly harass us in this situation. It is pretty easy to miss some patches, lost 
in all other mails.


I will review your patches. At first glance, it looks good for me. I will just 
enclose sample_conv_var2smp_str in the #if/#endif to avoid warning with older 
openssl versions.


--
Christopher Faulet

Re: [PATCH] Fix dumping of stick table entries for k/v pairs.

[Christopher Faulet]

Le 07/05/2020 à 15:27, Adis Nezirovic a écrit :

On 5/7/20 2:44 PM, William Lallemand wrote:

On Thu, May 07, 2020 at 02:39:30PM +0200, Christopher Faulet wrote:

Sorry for the delay and thanks for the reminder. I don't really know how stick
table works internally. But you should probably test the dict_entry pointer is
not NULL before accessing its value. It is how it is done in
stable_dump_entry_to_buffer(). If it ok for you, I can amend the commit to do
the same. Let me know.

Ah yes, that's obvious, please amend, I was trimming "unecessary"
temporary variables, and was left only with unguarded typecast :-)



Also, it should probably be tagged 'BUG/MEDIUM' instead of 'MEDIUM'

This one too, it is clearly a bug which causes user script in Lua to
fail during StickTable:dump() (and similar)




Merges now. Thanks !

--
Christopher Faulet

Re: [PATCH] Fix dumping of stick table entries for k/v pairs.

[Adis Nezirovic]

On 5/7/20 2:44 PM, William Lallemand wrote:

On Thu, May 07, 2020 at 02:39:30PM +0200, Christopher Faulet wrote:

Sorry for the delay and thanks for the reminder. I don't really know how stick
table works internally. But you should probably test the dict_entry pointer is
not NULL before accessing its value. It is how it is done in
stable_dump_entry_to_buffer(). If it ok for you, I can amend the commit to do
the same. Let me know.
Ah yes, that's obvious, please amend, I was trimming "unecessary" 
temporary variables, and was left only with unguarded typecast :-)




Also, it should probably be tagged 'BUG/MEDIUM' instead of 'MEDIUM'
This one too, it is clearly a bug which causes user script in Lua to 
fail during StickTable:dump() (and similar)



Best regards,
--
Adis Nezirovic
Software Engineer
HAProxy Technologies - Powering your uptime!
375 Totten Pond Road, Suite 302 | Waltham, MA 02451, US
+1 (844) 222-4340 | https://www.haproxy.com

Re: [PATCH] Fix dumping of stick table entries for k/v pairs.

[William Lallemand]

On Thu, May 07, 2020 at 02:39:30PM +0200, Christopher Faulet wrote:
> Le 07/05/2020 à 12:17, Adis Nezirovic a écrit :
> > Hello guys, any comments on the patch?
> > 
> > Bump, adding Thierry to the conversation.
> > 
> > 
> > Best regards,
> > 
> 
> Hi Adis,
> 
> Sorry for the delay and thanks for the reminder. I don't really know how 
> stick 
> table works internally. But you should probably test the dict_entry pointer 
> is 
> not NULL before accessing its value. It is how it is done in 
> stable_dump_entry_to_buffer(). If it ok for you, I can amend the commit to do 
> the same. Let me know.
> 

Hi,

Also, it should probably be tagged 'BUG/MEDIUM' instead of 'MEDIUM'

-- 
William Lallemand

Re: [PATCH] Fix dumping of stick table entries for k/v pairs.

[Christopher Faulet]

Le 07/05/2020 à 12:17, Adis Nezirovic a écrit :

Hello guys, any comments on the patch?

Bump, adding Thierry to the conversation.


Best regards,



Hi Adis,

Sorry for the delay and thanks for the reminder. I don't really know how stick 
table works internally. But you should probably test the dict_entry pointer is 
not NULL before accessing its value. It is how it is done in 
stable_dump_entry_to_buffer(). If it ok for you, I can amend the commit to do 
the same. Let me know.


--
Christopher Faulet

Re: [PATCH] MINOR: crypto: Add digest and hmac converters

[Tim Düsterhus]

Patrick,

Am 07.05.20 um 13:03 schrieb Patrick Gansterer:
> Hi,
> 
> On 22.04.20 18:30, Tim Düsterhus wrote:
>> I don't find anything to complain about now. I'll now leave it up to the
>> authority to either apply or complain.
> 
> How long does it usually take to get a response?

Usually not this long. If a patch of mine waits longer than 2 weeks I
usually ask again. See also:
https://github.com/haproxy/haproxy/blob/d888f0fc6ee63abf9dbdab49adf14b63f3ccb1ad/CONTRIBUTING#L164

I've added Willy back to the Cc, I already did for the email you just
replied to.

Best regards
Tim Düsterhus

Re: [PATCH] MINOR: crypto: Add digest and hmac converters

[Patrick Gansterer]

Hi,

On 22.04.20 18:30, Tim Düsterhus wrote:

I don't find anything to complain about now. I'll now leave it up to the
authority to either apply or complain.


How long does it usually take to get a response?

I posted a similar patch already 2 years ago and never got a response 
from the maintainers. It would be nice if it could make its way into 
2.2, since it's not invasive and has proper testing.

If gets rejected I'm ok too, but it would be nice to know at least.

- Patrick

Advertising on haproxy.org

[Julia Smith]

Hello haproxy.org team,

I'm a representative of the Profy1 outreach marketing team. Our company
would like to cooperate with you.

We are interested in publishing a guest/sponsored article with a permanent
do-follow link indexed by Google on your site.

We have different topics and would like to provide you with a relevant
article that will fit your site. Just let me know whether you are
interested or not.

And do you own any other sites? We'll be interested in them too. I'm
looking forward to your reply.

P.S. If you don’t accept contributed content, just click on the
"unsubscribe" button below and I won't try to reach you out any more.

Regards,
Julia Smith

You may unsubscribe

to stop receiving our emails.

Re: [PATCH] Fix dumping of stick table entries for k/v pairs.

[Adis Nezirovic]

Hello guys, any comments on the patch?

Bump, adding Thierry to the conversation.


Best regards,
--
Adis Nezirovic
Software Engineer
HAProxy Technologies - Powering your uptime!
375 Totten Pond Road, Suite 302 | Waltham, MA 02451, US
+1 (844) 222-4340 | https://www.haproxy.com

Re: [EXT] Re: about Warning: Setting tune.ssl.default-dh-param to 1024

[Willy Tarreau]

On Thu, May 07, 2020 at 10:20:15AM +0200, Remi Gacogne wrote:
> Hello,
> 
> On 5/7/20 12:01 AM, Lukas Tribus wrote:
> >> I'm fine with that, most people use at least a value of 2048 because of
> >> the warning, their modern distribution will probably deny a lower value,
> >> and we add this warning a long time ago.
> > 
> > I agree, we should default to 2048 and remove warnings.
> 
> I agree as well, let's use a more secure default and let those who need
> to support older clients do so explicitly.

Many thanks guys for your quick feedback. I'm going to do this and
update the doc then.

Cheers,
Willy

Re: [EXT] Re: about Warning: Setting tune.ssl.default-dh-param to 1024

[Remi Gacogne]

Hello,

On 5/7/20 12:01 AM, Lukas Tribus wrote:
>> I'm fine with that, most people use at least a value of 2048 because of
>> the warning, their modern distribution will probably deny a lower value,
>> and we add this warning a long time ago.
> 
> I agree, we should default to 2048 and remove warnings.

I agree as well, let's use a more secure default and let those who need
to support older clients do so explicitly.

Cheers,
-- 
Remi



signature.asc
Description: OpenPGP digital signature

Re: [tcp|http]-check expect status explained

[Aleksandar Lazic]

Hi Christopher.

On 07.05.20 07:55, Christopher Faulet wrote:
> Le 07/05/2020 à 00:06, Aleksandar Lazic a écrit :
>> On 07.05.20 00:02, Lukas Tribus wrote:
>>> On Wed, 6 May 2020 at 23:33, Aleksandar Lazic  wrote:

 Hi.

 The doc for [tcp|http]-check expect have some *-status arguments like 
 "L7OK", "L7OKC","L6OK" and "L4OK" and so on.

 In the whole documentation are this states not explained.
 I'm not sure in which chapter this states fit's, quick reminder 
 HTTP,global, logging, new chapter?
 My suggestion is to add "1.2." HTTP in HAProxy and explain how the htx 
 works and what layer 4+6+7 means, opinions.
>>>
>>> It's not in the configuration documentation, it's in the management doc:
>>>
>>> https://cbonte.github.io/haproxy-dconv/2.0/management.html#9.1
>>
>> Thanks, I don't look very often there but I should.
> 
> Hi Aleks,
> 
> You're right, it is not obvious. These status are reported in the stats and 
> are described in the management guide as Lukas said. But it is probably a 
> good idea to be more explicit. I slightly updated the configuration 
> documentation to not rely on internal names. I added a short description for 
> each status instead.

Cool thanks.

> Thanks,