[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20180918005256.7uutwda4s3ofoxpd@ast-mbp>
Date: Mon, 17 Sep 2018 17:52:59 -0700
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>
Cc: Jakub Kicinski <jakub.kicinski@...ronome.com>,
Daniel Borkmann <daniel@...earbox.net>,
Thomas Richter <tmricht@...ux.ibm.com>,
Hendrik Brueckner <brueckner@...ux.ibm.com>,
Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [RFC/fix] Re: libbpf build broken on musl libc (Alpine Linux)
On Mon, Sep 17, 2018 at 12:16:36PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Sep 13, 2018 at 11:56:31AM -0700, Alexei Starovoitov escreveu:
> > On Thu, Sep 13, 2018 at 03:32:40PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Please do some testing with my perf/libbpf+str_error_r branch, it has
> > > two patches to get this fixed, the one I sent and a prep one making
> > > libbpf link against libapi.
>
> > > [acme@...et perf]$ git log --oneline -2
> > > a7ab924b7fec (HEAD -> perf/urgent, acme.korg/perf/libbpf+str_error_r) tools lib bpf: Use str_error_r() to fix the build in Alpine Linux
> > > fb4a79e04c2b tools lib bpf: Build and link to tools/lib/api/
>
> > https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=perf/libbpf%2bstr_error_r&id=fb4a79e04c2b37ee873a3b31a3250925cf466fff
> > we cannot do this.
> > lib/api is GPL we cannot use it in LGPL library.
>
> So, look at this second attempt, are you ok with it?
>
> Builds with all the Alpine Linux test build containers and some more,
> still building with all the others:
>
> 1 alpine:3.4 : Ok gcc (Alpine 5.3.0) 5.3.0
> 2 alpine:3.5 : Ok gcc (Alpine 6.2.1) 6.2.1 20160822
> 3 alpine:3.6 : Ok gcc (Alpine 6.3.0) 6.3.0
> 4 alpine:3.7 : Ok gcc (Alpine 6.4.0) 6.4.0
> 5 alpine:3.8 : Ok gcc (Alpine 6.4.0) 6.4.0
> 6 alpine:edge : Ok gcc (Alpine 6.4.0) 6.4.0
> 7 amazonlinux:1 : Ok gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-28)
> 8 amazonlinux:2 : Ok gcc (GCC) 7.3.1 20180303 (Red Hat 7.3.1-5)
> 9 android-ndk:r12b-arm : Ok arm-linux-androideabi-gcc (GCC) 4.9.x 20150123 (prerelease)
> 10 android-ndk:r15c-arm : Ok arm-linux-androideabi-gcc (GCC) 4.9.x 20150123 (prerelease)
> 11 centos:5 : Ok gcc (GCC) 4.1.2 20080704 (Red Hat 4.1.2-55)
> 12 centos:6 : Ok gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-23)
> 13 centos:7 : Ok gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-28)
> 14 debian:7 : Ok gcc (Debian 4.7.2-5) 4.7.2
> 15 debian:8 : Ok gcc (Debian 4.9.2-10+deb8u1) 4.9.2
> 16 debian:9 : Ok gcc (Debian 6.3.0-18+deb9u1) 6.3.0 20170516
> 17 debian:experimental : Ok gcc (Debian 8.2.0-4) 8.2.0
> 18 debian:experimental-x-arm64 : Ok aarch64-linux-gnu-gcc (Debian 8.2.0-4) 8.2.0
> 19 debian:experimental-x-mips : Ok mips-linux-gnu-gcc (Debian 8.2.0-4) 8.2.0
>
> commit 1ca0d8249e5bd335b1c33a33569e4ed94025128e
> Author: Arnaldo Carvalho de Melo <acme@...hat.com>
> Date: Fri Sep 14 16:47:14 2018 -0300
>
> tools lib bpf: Provide wrapper for strerror_r to build in !_GNU_SOURCE systems
>
> Same problem that got fixed in a similar fashion in tools/perf/ in
> c8b5f2c96d1b ("tools: Introduce str_error_r()"), fix it in the same
> way, licensing needs to be sorted out to libbpf to use libapi, so,
> for this simple case, just get the same wrapper in tools/lib/bpf.
>
> This makes libbpf and its users (bpftool, selftests, perf) to build
> again in Alpine Linux 3.[45678] and edge.
>
> Cc: Adrian Hunter <adrian.hunter@...el.com>
> Cc: Alexei Starovoitov <ast@...nel.org>
> Cc: Daniel Borkmann <daniel@...earbox.net>
> Cc: David Ahern <dsahern@...il.com>
> Cc: Hendrik Brueckner <brueckner@...ux.ibm.com>
> Cc: Jakub Kicinski <jakub.kicinski@...ronome.com>
> Cc: Jiri Olsa <jolsa@...nel.org>
> Cc: Martin KaFai Lau <kafai@...com>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Cc: Quentin Monnet <quentin.monnet@...ronome.com>
> Cc: Thomas Richter <tmricht@...ux.ibm.com>
> Cc: Wang Nan <wangnan0@...wei.com>
> Cc: Yonghong Song <yhs@...com>
> Fixes: 1ce6a9fc1549 ("bpf: fix build error in libbpf with EXTRA_CFLAGS="-Wp, -D_FORTIFY_SOURCE=2 -O2"")
> Link: https://lkml.kernel.org/n/tip-i8ckf6s06e7tayw7xxhhhkux@git.kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
>
> diff --git a/tools/lib/bpf/Build b/tools/lib/bpf/Build
> index 13a861135127..6eb9bacd1948 100644
> --- a/tools/lib/bpf/Build
> +++ b/tools/lib/bpf/Build
> @@ -1 +1 @@
> -libbpf-y := libbpf.o bpf.o nlattr.o btf.o libbpf_errno.o
> +libbpf-y := libbpf.o bpf.o nlattr.o btf.o libbpf_errno.o str_error.o
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 2abd0f112627..bdb94939fd60 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -50,6 +50,7 @@
> #include "libbpf.h"
> #include "bpf.h"
> #include "btf.h"
> +#include "str_error.h"
>
> #ifndef EM_BPF
> #define EM_BPF 247
> @@ -469,7 +470,7 @@ static int bpf_object__elf_init(struct bpf_object *obj)
> obj->efile.fd = open(obj->path, O_RDONLY);
> if (obj->efile.fd < 0) {
> char errmsg[STRERR_BUFSIZE];
> - char *cp = strerror_r(errno, errmsg, sizeof(errmsg));
> + char *cp = str_error(errno, errmsg, sizeof(errmsg));
>
> pr_warning("failed to open %s: %s\n", obj->path, cp);
> return -errno;
> @@ -810,8 +811,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
> data->d_size, name, idx);
> if (err) {
> char errmsg[STRERR_BUFSIZE];
> - char *cp = strerror_r(-err, errmsg,
> - sizeof(errmsg));
> + char *cp = str_error(-err, errmsg, sizeof(errmsg));
>
> pr_warning("failed to alloc program %s (%s): %s",
> name, obj->path, cp);
> @@ -1140,7 +1140,7 @@ bpf_object__create_maps(struct bpf_object *obj)
>
> *pfd = bpf_create_map_xattr(&create_attr);
> if (*pfd < 0 && create_attr.btf_key_type_id) {
> - cp = strerror_r(errno, errmsg, sizeof(errmsg));
> + cp = str_error(errno, errmsg, sizeof(errmsg));
> pr_warning("Error in bpf_create_map_xattr(%s):%s(%d). Retrying without BTF.\n",
> map->name, cp, errno);
> create_attr.btf_fd = 0;
> @@ -1155,7 +1155,7 @@ bpf_object__create_maps(struct bpf_object *obj)
> size_t j;
>
> err = *pfd;
> - cp = strerror_r(errno, errmsg, sizeof(errmsg));
> + cp = str_error(errno, errmsg, sizeof(errmsg));
> pr_warning("failed to create map (name: '%s'): %s\n",
> map->name, cp);
> for (j = 0; j < i; j++)
> @@ -1339,7 +1339,7 @@ load_program(enum bpf_prog_type type, enum bpf_attach_type expected_attach_type,
> }
>
> ret = -LIBBPF_ERRNO__LOAD;
> - cp = strerror_r(errno, errmsg, sizeof(errmsg));
> + cp = str_error(errno, errmsg, sizeof(errmsg));
> pr_warning("load bpf program failed: %s\n", cp);
>
> if (log_buf && log_buf[0] != '\0') {
> @@ -1654,7 +1654,7 @@ static int check_path(const char *path)
>
> dir = dirname(dname);
> if (statfs(dir, &st_fs)) {
> - cp = strerror_r(errno, errmsg, sizeof(errmsg));
> + cp = str_error(errno, errmsg, sizeof(errmsg));
> pr_warning("failed to statfs %s: %s\n", dir, cp);
> err = -errno;
> }
> @@ -1690,7 +1690,7 @@ int bpf_program__pin_instance(struct bpf_program *prog, const char *path,
> }
>
> if (bpf_obj_pin(prog->instances.fds[instance], path)) {
> - cp = strerror_r(errno, errmsg, sizeof(errmsg));
> + cp = str_error(errno, errmsg, sizeof(errmsg));
> pr_warning("failed to pin program: %s\n", cp);
> return -errno;
> }
> @@ -1708,7 +1708,7 @@ static int make_dir(const char *path)
> err = -errno;
>
> if (err) {
> - cp = strerror_r(-err, errmsg, sizeof(errmsg));
> + cp = str_error(-err, errmsg, sizeof(errmsg));
> pr_warning("failed to mkdir %s: %s\n", path, cp);
> }
> return err;
> @@ -1770,7 +1770,7 @@ int bpf_map__pin(struct bpf_map *map, const char *path)
> }
>
> if (bpf_obj_pin(map->fd, path)) {
> - cp = strerror_r(errno, errmsg, sizeof(errmsg));
> + cp = str_error(errno, errmsg, sizeof(errmsg));
> pr_warning("failed to pin map: %s\n", cp);
> return -errno;
> }
> diff --git a/tools/lib/bpf/str_error.c b/tools/lib/bpf/str_error.c
> new file mode 100644
> index 000000000000..b8798114a357
> --- /dev/null
> +++ b/tools/lib/bpf/str_error.c
> @@ -0,0 +1,18 @@
> +// SPDX-License-Identifier: LGPL-2.1
> +#undef _GNU_SOURCE
> +#include <string.h>
> +#include <stdio.h>
> +#include "str_error.h"
> +
> +/*
> + * Wrapper to allow for building in non-GNU systems such as Alpine Linux's musl
> + * libc, while checking strerror_r() return to avoid having to check this in
> + * all places calling it.
> + */
> +char *str_error(int err, char *dst, int len)
> +{
> + int ret = strerror_r(err, dst, len);
> + if (ret)
> + snprintf(dst, len, "ERROR: strerror_r(%d)=%d", err, ret);
> + return dst;
> +}
> diff --git a/tools/lib/bpf/str_error.h b/tools/lib/bpf/str_error.h
> new file mode 100644
> index 000000000000..b9a22564ddc6
> --- /dev/null
> +++ b/tools/lib/bpf/str_error.h
> @@ -0,0 +1,6 @@
> +// SPDX-License-Identifier: GPL-2.1
LGPL-2.1 in the above?
The rest looks good to me.
Should we take it via bpf-next tree?
If you feel there is an urgency to fix musl build, we can take it via bpf tree too.
Jakub, thoughts? you've been messing with strerror last..
Powered by blists - more mailing lists