2022-08-08 16:43:30

by Daniel Borkmann

[permalink] [raw]
Subject: Re: [PATCH 4/4] build: Switch to new openssl API for test-libcrypto

Hi Arnaldo,

On 7/19/22 7:05 PM, Roberto Sassu wrote:
> Switch to new EVP API for detecting libcrypto, as Fedora 36 returns an
> error when it encounters the deprecated function MD5_Init() and the others.
> The error would be interpreted as missing libcrypto, while in reality it is
> not.
>
> Signed-off-by: Roberto Sassu <[email protected]>

Given rest of the tooling fixes from Andres Freund went via perf tree and the
below is perf related as well, I presume you'll pick this up, too?

[0] https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/log/?h=perf/core

> tools/build/feature/test-libcrypto.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/tools/build/feature/test-libcrypto.c b/tools/build/feature/test-libcrypto.c
> index a98174e0569c..bc34a5bbb504 100644
> --- a/tools/build/feature/test-libcrypto.c
> +++ b/tools/build/feature/test-libcrypto.c
> @@ -1,16 +1,23 @@
> // SPDX-License-Identifier: GPL-2.0
> +#include <openssl/evp.h>
> #include <openssl/sha.h>
> #include <openssl/md5.h>
>
> int main(void)
> {
> - MD5_CTX context;
> + EVP_MD_CTX *mdctx;
> unsigned char md[MD5_DIGEST_LENGTH + SHA_DIGEST_LENGTH];
> unsigned char dat[] = "12345";
> + unsigned int digest_len;
>
> - MD5_Init(&context);
> - MD5_Update(&context, &dat[0], sizeof(dat));
> - MD5_Final(&md[0], &context);
> + mdctx = EVP_MD_CTX_new();
> + if (!mdctx)
> + return 0;
> +
> + EVP_DigestInit_ex(mdctx, EVP_md5(), NULL);
> + EVP_DigestUpdate(mdctx, &dat[0], sizeof(dat));
> + EVP_DigestFinal_ex(mdctx, &md[0], &digest_len);
> + EVP_MD_CTX_free(mdctx);
>
> SHA1(&dat[0], sizeof(dat), &md[0]);
>
>


2022-08-08 18:56:53

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 4/4] build: Switch to new openssl API for test-libcrypto

Em Mon, Aug 08, 2022 at 06:14:48PM +0200, Daniel Borkmann escreveu:
> Hi Arnaldo,
>
> On 7/19/22 7:05 PM, Roberto Sassu wrote:
> > Switch to new EVP API for detecting libcrypto, as Fedora 36 returns an
> > error when it encounters the deprecated function MD5_Init() and the others.
> > The error would be interpreted as missing libcrypto, while in reality it is
> > not.
> >
> > Signed-off-by: Roberto Sassu <[email protected]>
>
> Given rest of the tooling fixes from Andres Freund went via perf tree and the
> below is perf related as well, I presume you'll pick this up, too?

Sure.

> [0] https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/log/?h=perf/core
>
> > tools/build/feature/test-libcrypto.c | 15 +++++++++++----
> > 1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/build/feature/test-libcrypto.c b/tools/build/feature/test-libcrypto.c
> > index a98174e0569c..bc34a5bbb504 100644
> > --- a/tools/build/feature/test-libcrypto.c
> > +++ b/tools/build/feature/test-libcrypto.c
> > @@ -1,16 +1,23 @@
> > // SPDX-License-Identifier: GPL-2.0
> > +#include <openssl/evp.h>
> > #include <openssl/sha.h>
> > #include <openssl/md5.h>
> > int main(void)
> > {
> > - MD5_CTX context;
> > + EVP_MD_CTX *mdctx;
> > unsigned char md[MD5_DIGEST_LENGTH + SHA_DIGEST_LENGTH];
> > unsigned char dat[] = "12345";
> > + unsigned int digest_len;
> > - MD5_Init(&context);
> > - MD5_Update(&context, &dat[0], sizeof(dat));
> > - MD5_Final(&md[0], &context);
> > + mdctx = EVP_MD_CTX_new();
> > + if (!mdctx)
> > + return 0;
> > +
> > + EVP_DigestInit_ex(mdctx, EVP_md5(), NULL);
> > + EVP_DigestUpdate(mdctx, &dat[0], sizeof(dat));
> > + EVP_DigestFinal_ex(mdctx, &md[0], &digest_len);
> > + EVP_MD_CTX_free(mdctx);
> > SHA1(&dat[0], sizeof(dat), &md[0]);
> >

--

- Arnaldo

2022-08-09 15:34:47

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 4/4] build: Switch to new openssl API for test-libcrypto

Em Mon, Aug 08, 2022 at 03:33:34PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Aug 08, 2022 at 06:14:48PM +0200, Daniel Borkmann escreveu:
> > Hi Arnaldo,
> >
> > On 7/19/22 7:05 PM, Roberto Sassu wrote:
> > > Switch to new EVP API for detecting libcrypto, as Fedora 36 returns an
> > > error when it encounters the deprecated function MD5_Init() and the others.
> > > The error would be interpreted as missing libcrypto, while in reality it is
> > > not.
> > >
> > > Signed-off-by: Roberto Sassu <[email protected]>
> >
> > Given rest of the tooling fixes from Andres Freund went via perf tree and the
> > below is perf related as well, I presume you'll pick this up, too?
>
> Sure.
>
> > [0] https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/log/?h=perf/core

So I fixed up the first one, minor fuzzes, the second I had to fix
conflicts with the patchset from Andres, ended up as below, will test
build it then in my container kit.

- Arnaldo

commit bea955a0256e20cc18e87087e42f2a903b9a8b84
Author: Roberto Sassu <[email protected]>
Date: Tue Jul 19 19:05:53 2022 +0200

bpftool: Complete libbfd feature detection

Commit 6e8ccb4f624a7 ("tools/bpf: properly account for libbfd variations")
sets the linking flags depending on which flavor of the libbfd feature was
detected.

However, the flavors except libbfd cannot be detected, as they are not in
the feature list.

Complete the list of features to detect by adding libbfd-liberty and
libbfd-liberty-z.

Committer notes:

Adjust conflict with with:

1e1613f64cc8a09d ("tools bpftool: Don't display disassembler-four-args feature test")
600b7b26c07a070d ("tools bpftool: Fix compilation error with new binutils")

Fixes: 6e8ccb4f624a73c5 ("tools/bpf: properly account for libbfd variations")
Signed-off-by: Roberto Sassu <[email protected]>
Cc: Alexei Starovoitov <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Andrii Nakryiko <[email protected]>
Cc: [email protected]
Cc: Daniel Borkmann <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: John Fastabend <[email protected]>
Cc: KP Singh <[email protected]>
Cc: [email protected]
Cc: Martin KaFai Lau <[email protected]>
Cc: Nathan Chancellor <[email protected]>
Cc: Nick Desaulniers <[email protected]>
Cc: Nick Terrell <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Quentin Monnet <[email protected]>
Cc: Song Liu <[email protected]>
Cc: Stanislav Fomichev <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 04d733e98bffbc08..9cc132277150c534 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -93,9 +93,11 @@ INSTALL ?= install
RM ?= rm -f

FEATURE_USER = .bpftool
-FEATURE_TESTS = libbfd disassembler-four-args disassembler-init-styled libcap \
+FEATURE_TESTS = libbfd libbfd-liberty libbfd-liberty-z
+ disassembler-four-args disassembler-init-styled libcap \
clang-bpf-co-re
-FEATURE_DISPLAY = libbfd libcap clang-bpf-co-re
+FEATURE_DISPLAY = libbfd libbfd-liberty libbfd-liberty-z
+ libcap clang-bpf-co-re

check_feat := 1
NON_CHECK_FEAT_TARGETS := clean uninstall doc doc-clean doc-install doc-uninstall

2022-08-09 15:36:55

by Quentin Monnet

[permalink] [raw]
Subject: Re: [PATCH 4/4] build: Switch to new openssl API for test-libcrypto

On 09/08/2022 16:15, Arnaldo Carvalho de Melo wrote:
> Em Mon, Aug 08, 2022 at 03:33:34PM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Mon, Aug 08, 2022 at 06:14:48PM +0200, Daniel Borkmann escreveu:
>>> Hi Arnaldo,
>>>
>>> On 7/19/22 7:05 PM, Roberto Sassu wrote:
>>>> Switch to new EVP API for detecting libcrypto, as Fedora 36 returns an
>>>> error when it encounters the deprecated function MD5_Init() and the others.
>>>> The error would be interpreted as missing libcrypto, while in reality it is
>>>> not.
>>>>
>>>> Signed-off-by: Roberto Sassu <[email protected]>
>>>
>>> Given rest of the tooling fixes from Andres Freund went via perf tree and the
>>> below is perf related as well, I presume you'll pick this up, too?
>>
>> Sure.
>>
>>> [0] https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/log/?h=perf/core
>
> So I fixed up the first one, minor fuzzes, the second I had to fix
> conflicts with the patchset from Andres, ended up as below, will test
> build it then in my container kit.
>
> - Arnaldo
>
> commit bea955a0256e20cc18e87087e42f2a903b9a8b84
> Author: Roberto Sassu <[email protected]>
> Date: Tue Jul 19 19:05:53 2022 +0200
>
> bpftool: Complete libbfd feature detection
>
> Commit 6e8ccb4f624a7 ("tools/bpf: properly account for libbfd variations")
> sets the linking flags depending on which flavor of the libbfd feature was
> detected.
>
> However, the flavors except libbfd cannot be detected, as they are not in
> the feature list.
>
> Complete the list of features to detect by adding libbfd-liberty and
> libbfd-liberty-z.
>
> Committer notes:
>
> Adjust conflict with with:
>
> 1e1613f64cc8a09d ("tools bpftool: Don't display disassembler-four-args feature test")
> 600b7b26c07a070d ("tools bpftool: Fix compilation error with new binutils")
>
> Fixes: 6e8ccb4f624a73c5 ("tools/bpf: properly account for libbfd variations")
> Signed-off-by: Roberto Sassu <[email protected]>
> Cc: Alexei Starovoitov <[email protected]>
> Cc: Andres Freund <[email protected]>
> Cc: Andrii Nakryiko <[email protected]>
> Cc: [email protected]
> Cc: Daniel Borkmann <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: John Fastabend <[email protected]>
> Cc: KP Singh <[email protected]>
> Cc: [email protected]
> Cc: Martin KaFai Lau <[email protected]>
> Cc: Nathan Chancellor <[email protected]>
> Cc: Nick Desaulniers <[email protected]>
> Cc: Nick Terrell <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Quentin Monnet <[email protected]>
> Cc: Song Liu <[email protected]>
> Cc: Stanislav Fomichev <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
>
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index 04d733e98bffbc08..9cc132277150c534 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -93,9 +93,11 @@ INSTALL ?= install
> RM ?= rm -f
>
> FEATURE_USER = .bpftool
> -FEATURE_TESTS = libbfd disassembler-four-args disassembler-init-styled libcap \
> +FEATURE_TESTS = libbfd libbfd-liberty libbfd-liberty-z
> + disassembler-four-args disassembler-init-styled libcap \
> clang-bpf-co-re
> -FEATURE_DISPLAY = libbfd libcap clang-bpf-co-re
> +FEATURE_DISPLAY = libbfd libbfd-liberty libbfd-liberty-z
> + libcap clang-bpf-co-re
>
> check_feat := 1
> NON_CHECK_FEAT_TARGETS := clean uninstall doc doc-clean doc-install doc-uninstall

The adjustment looks good, thanks Arnaldo!

2022-08-09 15:38:55

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 4/4] build: Switch to new openssl API for test-libcrypto

Em Tue, Aug 09, 2022 at 12:15:25PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Aug 08, 2022 at 03:33:34PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Mon, Aug 08, 2022 at 06:14:48PM +0200, Daniel Borkmann escreveu:
> > > Hi Arnaldo,
> > >
> > > On 7/19/22 7:05 PM, Roberto Sassu wrote:
> > > > Switch to new EVP API for detecting libcrypto, as Fedora 36 returns an
> > > > error when it encounters the deprecated function MD5_Init() and the others.
> > > > The error would be interpreted as missing libcrypto, while in reality it is
> > > > not.
> > > >
> > > > Signed-off-by: Roberto Sassu <[email protected]>
> > >
> > > Given rest of the tooling fixes from Andres Freund went via perf tree and the
> > > below is perf related as well, I presume you'll pick this up, too?
> >
> > Sure.
> >
> > > [0] https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/log/?h=perf/core
>
> So I fixed up the first one, minor fuzzes, the second I had to fix
> conflicts with the patchset from Andres, ended up as below, will test
> build it then in my container kit.

So I backtracked, the way it works needs further consideration with
regard to the patchkit from Andres, that is already upstream, so it
would be good for Roberto to take a look at what is in torvalds/master
now and see if we have to removed that styled thing from Andres.

Andres, if you could take a look at Roberto's patchkit as well that
would be great.

- Arnaldo

> commit bea955a0256e20cc18e87087e42f2a903b9a8b84
> Author: Roberto Sassu <[email protected]>
> Date: Tue Jul 19 19:05:53 2022 +0200
>
> bpftool: Complete libbfd feature detection
>
> Commit 6e8ccb4f624a7 ("tools/bpf: properly account for libbfd variations")
> sets the linking flags depending on which flavor of the libbfd feature was
> detected.
>
> However, the flavors except libbfd cannot be detected, as they are not in
> the feature list.
>
> Complete the list of features to detect by adding libbfd-liberty and
> libbfd-liberty-z.
>
> Committer notes:
>
> Adjust conflict with with:
>
> 1e1613f64cc8a09d ("tools bpftool: Don't display disassembler-four-args feature test")
> 600b7b26c07a070d ("tools bpftool: Fix compilation error with new binutils")
>
> Fixes: 6e8ccb4f624a73c5 ("tools/bpf: properly account for libbfd variations")
> Signed-off-by: Roberto Sassu <[email protected]>
> Cc: Alexei Starovoitov <[email protected]>
> Cc: Andres Freund <[email protected]>
> Cc: Andrii Nakryiko <[email protected]>
> Cc: [email protected]
> Cc: Daniel Borkmann <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: John Fastabend <[email protected]>
> Cc: KP Singh <[email protected]>
> Cc: [email protected]
> Cc: Martin KaFai Lau <[email protected]>
> Cc: Nathan Chancellor <[email protected]>
> Cc: Nick Desaulniers <[email protected]>
> Cc: Nick Terrell <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Quentin Monnet <[email protected]>
> Cc: Song Liu <[email protected]>
> Cc: Stanislav Fomichev <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
>
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index 04d733e98bffbc08..9cc132277150c534 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -93,9 +93,11 @@ INSTALL ?= install
> RM ?= rm -f
>
> FEATURE_USER = .bpftool
> -FEATURE_TESTS = libbfd disassembler-four-args disassembler-init-styled libcap \
> +FEATURE_TESTS = libbfd libbfd-liberty libbfd-liberty-z
> + disassembler-four-args disassembler-init-styled libcap \
> clang-bpf-co-re
> -FEATURE_DISPLAY = libbfd libcap clang-bpf-co-re
> +FEATURE_DISPLAY = libbfd libbfd-liberty libbfd-liberty-z
> + libcap clang-bpf-co-re
>
> check_feat := 1
> NON_CHECK_FEAT_TARGETS := clean uninstall doc doc-clean doc-install doc-uninstall

--

- Arnaldo

2022-08-09 17:56:14

by Andres Freund

[permalink] [raw]
Subject: Re: [PATCH 4/4] build: Switch to new openssl API for test-libcrypto

Hi,

On 2022-08-09 12:21:15 -0300, Arnaldo Carvalho de Melo wrote:
> So I backtracked, the way it works needs further consideration with
> regard to the patchkit from Andres, that is already upstream, so it
> would be good for Roberto to take a look at what is in torvalds/master
> now and see if we have to removed that styled thing from Andres.

Why would it have to be removed - seems to be fairly independent, leaving the
line conflicts aside? Or do you just mean folding it into one-big-test? If so,
that'd make sense, although I'm not sure how ready the infrastructure


FWIW, if I would have to maintain these, I'd probably change FEATURE_TESTS,
FEATURE_DISPLAY into one-item-per-line to make conflicts less common and
easier to resolve.


> Andres, if you could take a look at Roberto's patchkit as well that
> would be great.

I briefly scanned it, and the only real comment I have mirror's Quentin's,
namely that it'd be nice to avoid displaying more tests that don't tell the
user much.

Greetings,

Andres Freund

2022-08-09 19:33:04

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 4/4] build: Switch to new openssl API for test-libcrypto

Em Tue, Aug 09, 2022 at 10:00:34AM -0700, Andres Freund escreveu:
> Hi,
>
> On 2022-08-09 12:21:15 -0300, Arnaldo Carvalho de Melo wrote:
> > So I backtracked, the way it works needs further consideration with
> > regard to the patchkit from Andres, that is already upstream, so it
> > would be good for Roberto to take a look at what is in torvalds/master
> > now and see if we have to removed that styled thing from Andres.
>
> Why would it have to be removed - seems to be fairly independent, leaving the
> line conflicts aside? Or do you just mean folding it into one-big-test? If so,
> that'd make sense, although I'm not sure how ready the infrastructure

So below is the 3rd patch in Roberto's patchkit adapted, I removed the
FEATURE_CHECK_LDFLAGS-disassembler-init-styled setting as we now
automatically try with multiple sets of libraries, as with
disassembler-four-args.

- Arnaldo

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 23648ea54e8d3d2c..0661a1cf98556ed3 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -297,9 +297,6 @@ FEATURE_CHECK_LDFLAGS-libpython := $(PYTHON_EMBED_LDOPTS)

FEATURE_CHECK_LDFLAGS-libaio = -lrt

-FEATURE_CHECK_LDFLAGS-disassembler-four-args = -lbfd -lopcodes -ldl
-FEATURE_CHECK_LDFLAGS-disassembler-init-styled = -lbfd -lopcodes -ldl
-
CORE_CFLAGS += -fno-omit-frame-pointer
CORE_CFLAGS += -ggdb3
CORE_CFLAGS += -funwind-tables
@@ -329,8 +326,8 @@ ifneq ($(TCMALLOC),)
endif

ifeq ($(FEATURES_DUMP),)
-# We will display at the end of this Makefile.config, using $(call feature_display_entries)
-# As we may retry some feature detection here, see the disassembler-four-args case, for instance
+# We will display at the end of this Makefile.config, using $(call feature_display_entries),
+# as we may retry some feature detection here.
FEATURE_DISPLAY_DEFERRED := 1
include $(srctree)/tools/build/Makefile.feature
else
@@ -924,13 +921,9 @@ ifndef NO_LIBBFD

ifeq ($(feature-libbfd-liberty), 1)
EXTLIBS += -lbfd -lopcodes -liberty
- FEATURE_CHECK_LDFLAGS-disassembler-four-args += -liberty -ldl
- FEATURE_CHECK_LDFLAGS-disassembler-init-styled += -liberty -ldl
else
ifeq ($(feature-libbfd-liberty-z), 1)
EXTLIBS += -lbfd -lopcodes -liberty -lz
- FEATURE_CHECK_LDFLAGS-disassembler-four-args += -liberty -lz -ldl
- FEATURE_CHECK_LDFLAGS-disassembler-init-styled += -liberty -lz -ldl
endif
endif
$(call feature_check,disassembler-four-args)
@@ -1356,7 +1349,7 @@ endif

# re-generate FEATURE-DUMP as we may have called feature_check, found out
# extra libraries to add to LDFLAGS of some other test and then redo those
-# tests, see the block about libbfd, disassembler-four-args, for instance.
+# tests.
$(shell rm -f $(FEATURE_DUMP_FILENAME))
$(foreach feat,$(FEATURE_TESTS),$(shell echo "$(call feature_assign,$(feat))" >> $(FEATURE_DUMP_FILENAME)))


>
> FWIW, if I would have to maintain these, I'd probably change FEATURE_TESTS,
> FEATURE_DISPLAY into one-item-per-line to make conflicts less common and
> easier to resolve.
>
>
> > Andres, if you could take a look at Roberto's patchkit as well that
> > would be great.
>
> I briefly scanned it, and the only real comment I have mirror's Quentin's,
> namely that it'd be nice to avoid displaying more tests that don't tell the
> user much.
>
> Greetings,
>
> Andres Freund

--

- Arnaldo