lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 16 Jan 2023 09:20:39 -0800
From:   Ian Rogers <irogers@...gle.com>
To:     Jiri Olsa <olsajiri@...il.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <martin.lau@...ux.dev>,
        Song Liu <song@...nel.org>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        Stanislav Fomichev <sdf@...gle.com>,
        Hao Luo <haoluo@...gle.com>,
        Connor OBrien <connoro@...gle.com>,
        Nathan Chancellor <nathan@...nel.org>,
        Kumar Kartikeya Dwivedi <memxor@...il.com>,
        bpf@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] tools/resolve_btfids: Install subcmd headers

On Mon, Jan 16, 2023 at 3:22 AM Jiri Olsa <olsajiri@...il.com> wrote:
>
> On Wed, Jan 11, 2023 at 04:40:24PM -0800, Ian Rogers wrote:
> > Previously tools/lib/subcmd was added to the include path, switch to
> > installing the headers and then including from that directory. This
> > avoids dependencies on headers internal to tools/lib/subcmd. Add the
> > missing subcmd directory to the affected #include.
> >
> > Signed-off-by: Ian Rogers <irogers@...gle.com>
> > ---
> >  tools/bpf/resolve_btfids/Makefile | 19 ++++++++++++++-----
> >  tools/bpf/resolve_btfids/main.c   |  2 +-
> >  2 files changed, 15 insertions(+), 6 deletions(-)
>
> this depends on changes that went to Arnaldo's tree right?
> I can't apply this on bpf-next/master

Hmm.. sorry for that. I did the work on the master branch of
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

> >
> > diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> > index 19a3112e271a..de7d29cf43d6 100644
> > --- a/tools/bpf/resolve_btfids/Makefile
> > +++ b/tools/bpf/resolve_btfids/Makefile
> > @@ -35,21 +35,29 @@ SUBCMD_SRC := $(srctree)/tools/lib/subcmd/
> >  BPFOBJ     := $(OUTPUT)/libbpf/libbpf.a
> >  LIBBPF_OUT := $(abspath $(dir $(BPFOBJ)))/
> >  SUBCMDOBJ  := $(OUTPUT)/libsubcmd/libsubcmd.a
> > +SUBCMD_OUT := $(abspath $(dir $(SUBCMDOBJ)))/
> >
> >  LIBBPF_DESTDIR := $(LIBBPF_OUT)
> >  LIBBPF_INCLUDE := $(LIBBPF_DESTDIR)include
> >
> > +SUBCMD_DESTDIR := $(SUBCMD_OUT)
> > +SUBCMD_INCLUDE := $(SUBCMD_DESTDIR)include
> > +
> >  BINARY     := $(OUTPUT)/resolve_btfids
> >  BINARY_IN  := $(BINARY)-in.o
> >
> >  all: $(BINARY)
> >
> > +prepare: $(SUBCMDOBJ)
>
> do we need special target for that? we already have BPFOBJ dependency
> placed in the BINARY_IN as prereq

BPFOBJ is $(OUTPUT)/libbpf/libbpf.a which is needed at link time. The
prepare step is one we have elsewhere and it creates things like the
header files necessary to compile the C code, so we need it earlier is
the answer.

> why not place both as BINARY_IN prereq, or is there some other reason
> for new 'prepare' target?

I was trying to follow the convention elsewhere in the tree of having
a prepare target that does things like get the necessary header files
ready. Having prepare is useful because if an additional dependency is
added, then it just needs to be added to prepare. It could be tedious
to list all the dependencies for every C file, although Makefile.build
handles most of that. It isn't clear to me why $(BPFOBJ) is a
dependency of $(BINARY_IN) as it is already a dependency of $(BINARY).

Thanks,
Ian

> jirka
>
> > +
> >  $(OUTPUT) $(OUTPUT)/libsubcmd $(LIBBPF_OUT):
> >       $(call msg,MKDIR,,$@)
> >       $(Q)mkdir -p $(@)
> >
> >  $(SUBCMDOBJ): fixdep FORCE | $(OUTPUT)/libsubcmd
> > -     $(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(abspath $(dir $@))/ $(abspath $@)
> > +     $(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(SUBCMD_OUT) \
> > +                 DESTDIR=$(SUBCMD_DESTDIR) prefix= \
> > +                 $(abspath $@) install_headers
> >
> >  $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OUT)
> >       $(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC) OUTPUT=$(LIBBPF_OUT)    \
> > @@ -60,14 +68,14 @@ CFLAGS += -g \
> >            -I$(srctree)/tools/include \
> >            -I$(srctree)/tools/include/uapi \
> >            -I$(LIBBPF_INCLUDE) \
> > -          -I$(SUBCMD_SRC)
> > +          -I$(SUBCMD_INCLUDE)
> >
> >  LIBS = -lelf -lz
> >
> >  export srctree OUTPUT CFLAGS Q
> >  include $(srctree)/tools/build/Makefile.include
> >
> > -$(BINARY_IN): $(BPFOBJ) fixdep FORCE | $(OUTPUT)
> > +$(BINARY_IN): $(BPFOBJ) fixdep FORCE prepare | $(OUTPUT)
> >       $(Q)$(MAKE) $(build)=resolve_btfids
> >
> >  $(BINARY): $(BPFOBJ) $(SUBCMDOBJ) $(BINARY_IN)
> > @@ -79,7 +87,8 @@ clean_objects := $(wildcard $(OUTPUT)/*.o                \
> >                              $(OUTPUT)/.*.o.d             \
> >                              $(LIBBPF_OUT)                \
> >                              $(LIBBPF_DESTDIR)            \
> > -                            $(OUTPUT)/libsubcmd          \
> > +                            $(SUBCMD_OUT)                \
> > +                            $(SUBCMD_DESTDIR)            \
> >                              $(OUTPUT)/resolve_btfids)
> >
> >  ifneq ($(clean_objects),)
> > @@ -96,4 +105,4 @@ tags:
> >
> >  FORCE:
> >
> > -.PHONY: all FORCE clean tags
> > +.PHONY: all FORCE clean tags prepare
> > diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
> > index 80cd7843c677..77058174082d 100644
> > --- a/tools/bpf/resolve_btfids/main.c
> > +++ b/tools/bpf/resolve_btfids/main.c
> > @@ -75,7 +75,7 @@
> >  #include <linux/err.h>
> >  #include <bpf/btf.h>
> >  #include <bpf/libbpf.h>
> > -#include <parse-options.h>
> > +#include <subcmd/parse-options.h>
> >
> >  #define BTF_IDS_SECTION      ".BTF_ids"
> >  #define BTF_ID               "__BTF_ID__"
> > --
> > 2.39.0.314.g84b9a713c41-goog
> >

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ