* [PATCH bpf] tools/bpf: properly account for libbfd variations
@ 2019-01-15 19:59 Stanislav Fomichev
2019-01-15 21:38 ` Jakub Kicinski
0 siblings, 1 reply; 9+ messages in thread
From: Stanislav Fomichev @ 2019-01-15 19:59 UTC (permalink / raw)
To: netdev
Cc: davem, ast, daniel, jakub.kicinski, quentin.monnet,
Stanislav Fomichev
On some platforms, in order to link against libbfd, we need to
link against liberty and even possibly libz. Account for that
in the bpftool Makefile. We now have proper feature detection
for each case, so handle each one separately.
See recent commit 14541b1e7e72 ("perf build: Don't unconditionally link the
libbfd feature test to -liberty and -lz") where I fixed feature
detection.
Fixes: 29a9c10e4110 ("bpftool: make libbfd optional")
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/bpf/bpftool/Makefile | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 492f0f24e2d3..af9a25bf480d 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -92,10 +92,21 @@ BFD_SRCS = jit_disasm.c
SRCS = $(filter-out $(BFD_SRCS),$(wildcard *.c))
-ifeq ($(feature-libbfd),1)
+ifeq ($(feature-libbfd), 1)
+LIBS += -lbfd -ldl -lopcodes
+else
+ ifeq ($(feature-libbfd-liberty), 1)
+ LIBS += -lbfd -ldl -lopcodes -liberty
+ else
+ ifeq ($(feature-libbfd-liberty-z), 1)
+ LIBS += -lbfd -ldl -lopcodes -liberty -lz
+ endif
+ endif
+endif
+
+ifneq ($(filter -lbfd,$(EXTLIBS)),)
CFLAGS += -DHAVE_LIBBFD_SUPPORT
SRCS += $(BFD_SRCS)
-LIBS += -lbfd -lopcodes
endif
OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH bpf] tools/bpf: properly account for libbfd variations
2019-01-15 19:59 [PATCH bpf] tools/bpf: properly account for libbfd variations Stanislav Fomichev
@ 2019-01-15 21:38 ` Jakub Kicinski
2019-01-15 21:47 ` Stanislav Fomichev
2019-01-15 21:52 ` [PATCH bpf v2] " Stanislav Fomichev
0 siblings, 2 replies; 9+ messages in thread
From: Jakub Kicinski @ 2019-01-15 21:38 UTC (permalink / raw)
To: Stanislav Fomichev; +Cc: netdev, davem, ast, daniel, quentin.monnet
On Tue, 15 Jan 2019 11:59:53 -0800, Stanislav Fomichev wrote:
> On some platforms, in order to link against libbfd, we need to
> link against liberty and even possibly libz. Account for that
> in the bpftool Makefile. We now have proper feature detection
> for each case, so handle each one separately.
>
> See recent commit 14541b1e7e72 ("perf build: Don't unconditionally link the
> libbfd feature test to -liberty and -lz") where I fixed feature
> detection.
>
> Fixes: 29a9c10e4110 ("bpftool: make libbfd optional")
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
Minor nits below, in any case:
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Thanks for making bpftool build! :)
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index 492f0f24e2d3..af9a25bf480d 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -92,10 +92,21 @@ BFD_SRCS = jit_disasm.c
>
> SRCS = $(filter-out $(BFD_SRCS),$(wildcard *.c))
>
> -ifeq ($(feature-libbfd),1)
> +ifeq ($(feature-libbfd), 1)
nit: no space there is more common
$ git grep 'ifeq' | grep ',[^ ]' | wc -l
482
$ git grep 'ifeq' | grep ', ' | wc -l
136
> +LIBS += -lbfd -ldl -lopcodes
nit: should this be indented?
> +else
> + ifeq ($(feature-libbfd-liberty), 1)
> + LIBS += -lbfd -ldl -lopcodes -liberty
> + else
> + ifeq ($(feature-libbfd-liberty-z), 1)
> + LIBS += -lbfd -ldl -lopcodes -liberty -lz
> + endif
Would this syntax:
ifeq ($(feature-libbfd),1)
LIBS += ..
else ifeq ($(feature-libbfd-liberty),1)
LIBS += ..
else ifeq ($(feature-libbfd-liberty-z),1)
LIBS += ..
endif
Not work?
https://www.gnu.org/software/make/manual/html_node/Conditional-Syntax.html
I don't think I've ever tried, but looks more concise..
> + endif
> +endif
> +
> +ifneq ($(filter -lbfd,$(EXTLIBS)),)
> CFLAGS += -DHAVE_LIBBFD_SUPPORT
> SRCS += $(BFD_SRCS)
> -LIBS += -lbfd -lopcodes
> endif
>
> OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH bpf] tools/bpf: properly account for libbfd variations
2019-01-15 21:38 ` Jakub Kicinski
@ 2019-01-15 21:47 ` Stanislav Fomichev
2019-01-15 21:52 ` [PATCH bpf v2] " Stanislav Fomichev
1 sibling, 0 replies; 9+ messages in thread
From: Stanislav Fomichev @ 2019-01-15 21:47 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Stanislav Fomichev, netdev, davem, ast, daniel, quentin.monnet
On 01/15, Jakub Kicinski wrote:
> On Tue, 15 Jan 2019 11:59:53 -0800, Stanislav Fomichev wrote:
> > On some platforms, in order to link against libbfd, we need to
> > link against liberty and even possibly libz. Account for that
> > in the bpftool Makefile. We now have proper feature detection
> > for each case, so handle each one separately.
> >
> > See recent commit 14541b1e7e72 ("perf build: Don't unconditionally link the
> > libbfd feature test to -liberty and -lz") where I fixed feature
> > detection.
> >
> > Fixes: 29a9c10e4110 ("bpftool: make libbfd optional")
> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
>
> Minor nits below, in any case:
>
> Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>
> Thanks for making bpftool build! :)
>
> > diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> > index 492f0f24e2d3..af9a25bf480d 100644
> > --- a/tools/bpf/bpftool/Makefile
> > +++ b/tools/bpf/bpftool/Makefile
> > @@ -92,10 +92,21 @@ BFD_SRCS = jit_disasm.c
> >
> > SRCS = $(filter-out $(BFD_SRCS),$(wildcard *.c))
> >
> > -ifeq ($(feature-libbfd),1)
> > +ifeq ($(feature-libbfd), 1)
>
> nit: no space there is more common
>
> $ git grep 'ifeq' | grep ',[^ ]' | wc -l
> 482
> $ git grep 'ifeq' | grep ', ' | wc -l
> 136
>
>
> > +LIBS += -lbfd -ldl -lopcodes
>
> nit: should this be indented?
>
> > +else
> > + ifeq ($(feature-libbfd-liberty), 1)
> > + LIBS += -lbfd -ldl -lopcodes -liberty
> > + else
> > + ifeq ($(feature-libbfd-liberty-z), 1)
> > + LIBS += -lbfd -ldl -lopcodes -liberty -lz
> > + endif
>
> Would this syntax:
>
> ifeq ($(feature-libbfd),1)
> LIBS += ..
> else ifeq ($(feature-libbfd-liberty),1)
> LIBS += ..
> else ifeq ($(feature-libbfd-liberty-z),1)
> LIBS += ..
> endif
>
> Not work?
I does seem to work :-) This is the first time I see it being written that
way. (but your link indeed shows that this syntax is supported)
I'll post v2 shortly with all the nits addressed. Thanks for a quick
review!
>
> https://www.gnu.org/software/make/manual/html_node/Conditional-Syntax.html
>
> I don't think I've ever tried, but looks more concise..
>
> > + endif
> > +endif
> > +
> > +ifneq ($(filter -lbfd,$(EXTLIBS)),)
> > CFLAGS += -DHAVE_LIBBFD_SUPPORT
> > SRCS += $(BFD_SRCS)
> > -LIBS += -lbfd -lopcodes
> > endif
> >
> > OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
>
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH bpf v2] tools/bpf: properly account for libbfd variations
2019-01-15 21:38 ` Jakub Kicinski
2019-01-15 21:47 ` Stanislav Fomichev
@ 2019-01-15 21:52 ` Stanislav Fomichev
2019-01-15 21:55 ` Jakub Kicinski
1 sibling, 1 reply; 9+ messages in thread
From: Stanislav Fomichev @ 2019-01-15 21:52 UTC (permalink / raw)
To: netdev
Cc: davem, ast, daniel, jakub.kicinski, quentin.monnet,
Stanislav Fomichev
On some platforms, in order to link against libbfd, we need to
link against liberty and even possibly libz. Account for that
in the bpftool Makefile. We now have proper feature detection
for each case, so handle each one separately.
See recent commit 14541b1e7e72 ("perf build: Don't unconditionally link the
libbfd feature test to -liberty and -lz") where I fixed feature
detection.
v2 (addressed Jakub's nits):
* better syntax for 'else ifeq'
* no space between ifeq args
Fixes: 29a9c10e4110 ("bpftool: make libbfd optional")
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/bpf/bpftool/Makefile | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 492f0f24e2d3..fbf7e62a5b86 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -93,9 +93,16 @@ BFD_SRCS = jit_disasm.c
SRCS = $(filter-out $(BFD_SRCS),$(wildcard *.c))
ifeq ($(feature-libbfd),1)
+ LIBS += -lbfd -ldl -lopcodes
+else ifeq ($(feature-libbfd-liberty),1)
+ LIBS += -lbfd -ldl -lopcodes -liberty
+else ifeq ($(feature-libbfd-liberty-z),1)
+ LIBS += -lbfd -ldl -lopcodes -liberty -lz
+endif
+
+ifneq ($(filter -lbfd,$(EXTLIBS)),)
CFLAGS += -DHAVE_LIBBFD_SUPPORT
SRCS += $(BFD_SRCS)
-LIBS += -lbfd -lopcodes
endif
OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH bpf v2] tools/bpf: properly account for libbfd variations
2019-01-15 21:52 ` [PATCH bpf v2] " Stanislav Fomichev
@ 2019-01-15 21:55 ` Jakub Kicinski
2019-01-15 22:00 ` Stanislav Fomichev
2019-01-15 22:03 ` [PATCH bpf v3] " Stanislav Fomichev
0 siblings, 2 replies; 9+ messages in thread
From: Jakub Kicinski @ 2019-01-15 21:55 UTC (permalink / raw)
To: Stanislav Fomichev; +Cc: netdev, davem, ast, daniel, quentin.monnet
On Tue, 15 Jan 2019 13:52:48 -0800, Stanislav Fomichev wrote:
> On some platforms, in order to link against libbfd, we need to
> link against liberty and even possibly libz. Account for that
> in the bpftool Makefile. We now have proper feature detection
> for each case, so handle each one separately.
>
> See recent commit 14541b1e7e72 ("perf build: Don't unconditionally link the
> libbfd feature test to -liberty and -lz") where I fixed feature
> detection.
>
> v2 (addressed Jakub's nits):
> * better syntax for 'else ifeq'
> * no space between ifeq args
>
> Fixes: 29a9c10e4110 ("bpftool: make libbfd optional")
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
> tools/bpf/bpftool/Makefile | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index 492f0f24e2d3..fbf7e62a5b86 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -93,9 +93,16 @@ BFD_SRCS = jit_disasm.c
> SRCS = $(filter-out $(BFD_SRCS),$(wildcard *.c))
>
> ifeq ($(feature-libbfd),1)
> + LIBS += -lbfd -ldl -lopcodes
> +else ifeq ($(feature-libbfd-liberty),1)
> + LIBS += -lbfd -ldl -lopcodes -liberty
> +else ifeq ($(feature-libbfd-liberty-z),1)
> + LIBS += -lbfd -ldl -lopcodes -liberty -lz
> +endif
> +
> +ifneq ($(filter -lbfd,$(EXTLIBS)),)
Ah, I just noticed now that you use EXTLIBS here, not LIBS, is that on
purpose?
> CFLAGS += -DHAVE_LIBBFD_SUPPORT
> SRCS += $(BFD_SRCS)
> -LIBS += -lbfd -lopcodes
> endif
>
> OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH bpf v2] tools/bpf: properly account for libbfd variations
2019-01-15 21:55 ` Jakub Kicinski
@ 2019-01-15 22:00 ` Stanislav Fomichev
2019-01-15 22:03 ` [PATCH bpf v3] " Stanislav Fomichev
1 sibling, 0 replies; 9+ messages in thread
From: Stanislav Fomichev @ 2019-01-15 22:00 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Stanislav Fomichev, netdev, davem, ast, daniel, quentin.monnet
On 01/15, Jakub Kicinski wrote:
> On Tue, 15 Jan 2019 13:52:48 -0800, Stanislav Fomichev wrote:
> > On some platforms, in order to link against libbfd, we need to
> > link against liberty and even possibly libz. Account for that
> > in the bpftool Makefile. We now have proper feature detection
> > for each case, so handle each one separately.
> >
> > See recent commit 14541b1e7e72 ("perf build: Don't unconditionally link the
> > libbfd feature test to -liberty and -lz") where I fixed feature
> > detection.
> >
> > v2 (addressed Jakub's nits):
> > * better syntax for 'else ifeq'
> > * no space between ifeq args
> >
> > Fixes: 29a9c10e4110 ("bpftool: make libbfd optional")
> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > ---
> > tools/bpf/bpftool/Makefile | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> > index 492f0f24e2d3..fbf7e62a5b86 100644
> > --- a/tools/bpf/bpftool/Makefile
> > +++ b/tools/bpf/bpftool/Makefile
> > @@ -93,9 +93,16 @@ BFD_SRCS = jit_disasm.c
> > SRCS = $(filter-out $(BFD_SRCS),$(wildcard *.c))
> >
> > ifeq ($(feature-libbfd),1)
> > + LIBS += -lbfd -ldl -lopcodes
> > +else ifeq ($(feature-libbfd-liberty),1)
> > + LIBS += -lbfd -ldl -lopcodes -liberty
> > +else ifeq ($(feature-libbfd-liberty-z),1)
> > + LIBS += -lbfd -ldl -lopcodes -liberty -lz
> > +endif
> > +
> > +ifneq ($(filter -lbfd,$(EXTLIBS)),)
>
> Ah, I just noticed now that you use EXTLIBS here, not LIBS, is that on
> purpose?
Nice catch! Should be LIBS, probably copy-pasted it from somewhere :-(
>
> > CFLAGS += -DHAVE_LIBBFD_SUPPORT
> > SRCS += $(BFD_SRCS)
> > -LIBS += -lbfd -lopcodes
> > endif
> >
> > OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
>
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH bpf v3] tools/bpf: properly account for libbfd variations
2019-01-15 21:55 ` Jakub Kicinski
2019-01-15 22:00 ` Stanislav Fomichev
@ 2019-01-15 22:03 ` Stanislav Fomichev
2019-01-15 22:06 ` Jakub Kicinski
2019-01-15 23:55 ` Daniel Borkmann
1 sibling, 2 replies; 9+ messages in thread
From: Stanislav Fomichev @ 2019-01-15 22:03 UTC (permalink / raw)
To: netdev
Cc: davem, ast, daniel, jakub.kicinski, quentin.monnet,
Stanislav Fomichev
On some platforms, in order to link against libbfd, we need to
link against liberty and even possibly libz. Account for that
in the bpftool Makefile. We now have proper feature detection
for each case, so handle each one separately.
See recent commit 14541b1e7e72 ("perf build: Don't unconditionally link the
libbfd feature test to -liberty and -lz") where I fixed feature
detection.
v2 (addressed Jakub's nits):
* better syntax for 'else ifeq'
* no space between ifeq args
v3:
* use LIBS, not EXTLIBS for -DHAVE_LIBBFD_SUPPORT
Fixes: 29a9c10e4110 ("bpftool: make libbfd optional")
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/bpf/bpftool/Makefile | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 492f0f24e2d3..4ad1f0894d53 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -93,9 +93,16 @@ BFD_SRCS = jit_disasm.c
SRCS = $(filter-out $(BFD_SRCS),$(wildcard *.c))
ifeq ($(feature-libbfd),1)
+ LIBS += -lbfd -ldl -lopcodes
+else ifeq ($(feature-libbfd-liberty),1)
+ LIBS += -lbfd -ldl -lopcodes -liberty
+else ifeq ($(feature-libbfd-liberty-z),1)
+ LIBS += -lbfd -ldl -lopcodes -liberty -lz
+endif
+
+ifneq ($(filter -lbfd,$(LIBS)),)
CFLAGS += -DHAVE_LIBBFD_SUPPORT
SRCS += $(BFD_SRCS)
-LIBS += -lbfd -lopcodes
endif
OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH bpf v3] tools/bpf: properly account for libbfd variations
2019-01-15 22:03 ` [PATCH bpf v3] " Stanislav Fomichev
@ 2019-01-15 22:06 ` Jakub Kicinski
2019-01-15 23:55 ` Daniel Borkmann
1 sibling, 0 replies; 9+ messages in thread
From: Jakub Kicinski @ 2019-01-15 22:06 UTC (permalink / raw)
To: Stanislav Fomichev; +Cc: netdev, davem, ast, daniel, quentin.monnet
On Tue, 15 Jan 2019 14:03:27 -0800, Stanislav Fomichev wrote:
> On some platforms, in order to link against libbfd, we need to
> link against liberty and even possibly libz. Account for that
> in the bpftool Makefile. We now have proper feature detection
> for each case, so handle each one separately.
>
> See recent commit 14541b1e7e72 ("perf build: Don't unconditionally link the
> libbfd feature test to -liberty and -lz") where I fixed feature
> detection.
>
> v2 (addressed Jakub's nits):
> * better syntax for 'else ifeq'
> * no space between ifeq args
> v3:
> * use LIBS, not EXTLIBS for -DHAVE_LIBBFD_SUPPORT
>
> Fixes: 29a9c10e4110 ("bpftool: make libbfd optional")
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Thanks!!
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH bpf v3] tools/bpf: properly account for libbfd variations
2019-01-15 22:03 ` [PATCH bpf v3] " Stanislav Fomichev
2019-01-15 22:06 ` Jakub Kicinski
@ 2019-01-15 23:55 ` Daniel Borkmann
1 sibling, 0 replies; 9+ messages in thread
From: Daniel Borkmann @ 2019-01-15 23:55 UTC (permalink / raw)
To: Stanislav Fomichev, netdev; +Cc: davem, ast, jakub.kicinski, quentin.monnet
On 01/15/2019 11:03 PM, Stanislav Fomichev wrote:
> On some platforms, in order to link against libbfd, we need to
> link against liberty and even possibly libz. Account for that
> in the bpftool Makefile. We now have proper feature detection
> for each case, so handle each one separately.
>
> See recent commit 14541b1e7e72 ("perf build: Don't unconditionally link the
> libbfd feature test to -liberty and -lz") where I fixed feature
> detection.
>
> v2 (addressed Jakub's nits):
> * better syntax for 'else ifeq'
> * no space between ifeq args
> v3:
> * use LIBS, not EXTLIBS for -DHAVE_LIBBFD_SUPPORT
>
> Fixes: 29a9c10e4110 ("bpftool: make libbfd optional")
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
Applied, thanks!
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-01-15 23:55 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-15 19:59 [PATCH bpf] tools/bpf: properly account for libbfd variations Stanislav Fomichev
2019-01-15 21:38 ` Jakub Kicinski
2019-01-15 21:47 ` Stanislav Fomichev
2019-01-15 21:52 ` [PATCH bpf v2] " Stanislav Fomichev
2019-01-15 21:55 ` Jakub Kicinski
2019-01-15 22:00 ` Stanislav Fomichev
2019-01-15 22:03 ` [PATCH bpf v3] " Stanislav Fomichev
2019-01-15 22:06 ` Jakub Kicinski
2019-01-15 23:55 ` Daniel Borkmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).