tools: bpf: standardize make

PATCH In review Jiri Benc 4 participants 13 messages Mar 8, 2018 – Mar 9, 2018

Subsystems (4)

Networking Network drivers BPF bpftool

lore

Share & tools

lore

Jiri Benc

author

2018-03-08 22:00 UTC #0 lore raw

cover tools: bpf: standardize make

Currently, 'make bpf' in the tools/ directory does not provide the standard quiet output except for bpftool (which is however listed with a wrong directory). Worse, it does not respect the build output directory.

The 'make bpf_install' does not work as one would expect, either. It installs unconditionally to /usr/bin without respecting DESTDIR and prefix.

This patchset improves that behavior.

Jiri Benc (7):
tools: bpftool: silence 'missing initializer' warnings
tools: bpf: respect output directory during build
tools: bpf: consistent make bpf_install
tools: bpf: make install should build first
tools: bpf: call descend in Makefile
tools: bpf: respect quiet/verbose build
tools: bpf: silence make by not deleting intermediate file

 tools/bpf/Makefile         | 74 +++++++++++++++++++++++++++++++---------------
 tools/bpf/bpftool/Makefile |  2 +-
 2 files changed, 51 insertions(+), 25 deletions(-)

Signature · 1 line

1.8.3.1

Jiri Benc

author

2018-03-08 22:00 UTC #1 lore raw

PATCH 1/7 tools: bpftool: silence 'missing initializer' warnings

When building bpf tool, gcc emits piles of warnings:

prog.c: In function ‘prog_fd_by_tag’:
prog.c:101:9: warning: missing initializer for field ‘type’ of ‘struct bpf_prog_info’ [-Wmissing-field-initializers]

  struct bpf_prog_info info = {};
         ^
In file included from /home/storage/jbenc/git/net-next/tools/lib/bpf/bpf.h:26:0,
                 from prog.c:47:
/home/storage/jbenc/git/net-next/tools/include/uapi/linux/bpf.h:925:8: note: ‘type’ declared here
  __u32 type;
        ^

As these warnings are not useful, switch them off.

Signed-off-by Jiri Benc <[email protected]>

---
 tools/bpf/bpftool/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tools/bpf/bpftool/Makefile +1 -1

index 26901ec87361..4c2867481f5c 100644
@@ -38,7 +38,7 @@ bash_compdir ?= /usr/share/bash-completion/completions
 CC = gcc
 
 CFLAGS += -O2
-CFLAGS += -W -Wall -Wextra -Wno-unused-parameter -Wshadow
+CFLAGS += -W -Wall -Wextra -Wno-unused-parameter -Wshadow -Wno-missing-field-initializers
 CFLAGS += -DPACKAGE='"bpftool"' -D__EXPORTED_HEADERS__ -I$(srctree)/tools/include/uapi -I$(srctree)/tools/include -I$(srctree)/tools/lib/bpf -I$(srctree)/kernel/bpf/
 CFLAGS += -DBPFTOOL_VERSION='"$(BPFTOOL_VERSION)"'
 LIBS = -lelf -lbfd -lopcodes $(LIBBPF)

Signature · 1 line

1.8.3.1

Jakub Kicinski

2018-03-09 07:38 UTC #2 lore raw

Re: [PATCH bpf-next 1/7] tools: bpftool: silence 'missing initializer' warnings

Quoted 12 lines · Jiri Benc

When building bpf tool, gcc emits piles of warnings:
prog.c: In function ‘prog_fd_by_tag’:
prog.c:101:9: warning: missing initializer for field ‘type’ of ‘struct bpf_prog_info’ [-Wmissing-field-initializers]
  struct bpf_prog_info info = {};
         ^
In file included from /home/storage/jbenc/git/net-next/tools/lib/bpf/bpf.h:26:0,
                 from prog.c:47:
/home/storage/jbenc/git/net-next/tools/include/uapi/linux/bpf.h:925:8: note: ‘type’ declared here
  __u32 type;
        ^

Quoted 3 lines

As these warnings are not useful, switch them off.
Signed-off-by Jiri Benc <[email protected]>

FWIW I couldn't reproduce this one. Out of curiosity what GCC did you use?

Jiri Benc

author

2018-03-09 08:42 UTC #3 lore raw

Re: [PATCH bpf-next 1/7] tools: bpftool: silence 'missing initializer' warnings

Quoted 2 lines · Jakub Kicinski

FWIW I couldn't reproduce this one. Out of curiosity what GCC did you use?

gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-16)

Jiri

Jiri Benc

author

2018-03-08 22:00 UTC #4 lore raw

PATCH 2/7 tools: bpf: respect output directory during build

In reply to Jiri Benc

Currently, the programs under tools/bpf (with the notable exception of bpftool) do not respect the output directory (make O=dir). Fix that.

Signed-off-by Jiri Benc <[email protected]>

---
 tools/bpf/Makefile | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

tools/bpf/Makefile +22 ~~-16~~

index c8ec0ae16bf0..e7b15967492e 100644
@@ -1,4 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
+include ../scripts/Makefile.include
+
 prefix = /usr
 
 CC = gcc

@@ -7,7 +9,7 @@ YACC = bison
 MAKE = make
 
 CFLAGS += -Wall -O2
-CFLAGS += -D__EXPORTED_HEADERS__ -I../../include/uapi -I../../include
+CFLAGS += -D__EXPORTED_HEADERS__ -I$(srctree)/include/uapi -I$(srctree)/include
 
 ifeq ($(srctree),)
 srctree := $(patsubst %/,%,$(dir $(CURDIR)))

@@ -38,32 +40,36 @@ ifeq ($(feature-disassembler-four-args), 1)
 CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
 endif
 
-%.yacc.c: %.y
+$(OUTPUT)%.yacc.c: $(srctree)/tools/bpf/%.y
 	$(YACC) -o $@ -d $<
 
-%.lex.c: %.l
+$(OUTPUT)%.lex.c: $(srctree)/tools/bpf/%.l
 	$(LEX) -o $@ $<
 
-all: bpf_jit_disasm bpf_dbg bpf_asm bpftool
+$(OUTPUT)%.o: $(srctree)/tools/bpf/%.c
+	$(COMPILE.c) -o $@ $<
+
+all: $(OUTPUT)bpf_jit_disasm $(OUTPUT)bpf_dbg $(OUTPUT)bpf_asm bpftool
 
-bpf_jit_disasm : CFLAGS += -DPACKAGE='bpf_jit_disasm'
-bpf_jit_disasm : LDLIBS = -lopcodes -lbfd -ldl
-bpf_jit_disasm : bpf_jit_disasm.o
+$(OUTPUT)bpf_jit_disasm: CFLAGS += -DPACKAGE='bpf_jit_disasm'
+$(OUTPUT)bpf_jit_disasm: LDLIBS = -lopcodes -lbfd -ldl
+$(OUTPUT)bpf_jit_disasm: $(OUTPUT)bpf_jit_disasm.o
 
-bpf_dbg : LDLIBS = -lreadline
-bpf_dbg : bpf_dbg.o
+$(OUTPUT)bpf_dbg: LDLIBS = -lreadline
+$(OUTPUT)bpf_dbg: $(OUTPUT)bpf_dbg.o
 
-bpf_asm : LDLIBS =
-bpf_asm : bpf_asm.o bpf_exp.yacc.o bpf_exp.lex.o
-bpf_exp.lex.o : bpf_exp.yacc.c
+$(OUTPUT)bpf_asm: LDLIBS =
+$(OUTPUT)bpf_asm: $(OUTPUT)bpf_asm.o $(OUTPUT)bpf_exp.yacc.o $(OUTPUT)bpf_exp.lex.o
+$(OUTPUT)bpf_exp.lex.o: $(OUTPUT)bpf_exp.yacc.c
 
 clean: bpftool_clean
-	rm -rf *.o bpf_jit_disasm bpf_dbg bpf_asm bpf_exp.yacc.* bpf_exp.lex.*
+	rm -rf $(OUTPUT)*.o $(OUTPUT)bpf_jit_disasm $(OUTPUT)bpf_dbg \
+	       $(OUTPUT)bpf_asm $(OUTPUT)bpf_exp.yacc.* $(OUTPUT)bpf_exp.lex.*
 
 install: bpftool_install
-	install bpf_jit_disasm $(prefix)/bin/bpf_jit_disasm
-	install bpf_dbg $(prefix)/bin/bpf_dbg
-	install bpf_asm $(prefix)/bin/bpf_asm
+	install $(OUTPUT)bpf_jit_disasm $(prefix)/bin/bpf_jit_disasm
+	install $(OUTPUT)bpf_dbg $(prefix)/bin/bpf_dbg
+	install $(OUTPUT)bpf_asm $(prefix)/bin/bpf_asm
 
 bpftool:
 	$(MAKE) -C bpftool

Signature · 1 line

1.8.3.1

Jiri Benc

author

2018-03-08 22:00 UTC #5 lore raw

PATCH 3/7 tools: bpf: consistent make bpf_install

In reply to Jiri Benc

Currently, make bpf_install in tools/ does not respect DESTDIR. Moreover, it installs to /usr/bin/ unconditionally.

Let it respect DESTDIR and allow prefix to be specified. Also, to be more consistent with bpftool and with the usual customs, default the prefix to /usr/local instead of /usr.

Signed-off-by Jiri Benc <[email protected]>

---
 tools/bpf/Makefile | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

tools/bpf/Makefile +6 -4

index e7b15967492e..c42ca24a072d 100644
@@ -1,12 +1,13 @@
 # SPDX-License-Identifier: GPL-2.0
 include ../scripts/Makefile.include
 
-prefix = /usr
+prefix ?= /usr/local
 
 CC = gcc
 LEX = flex
 YACC = bison
 MAKE = make
+INSTALL ?= install
 
 CFLAGS += -Wall -O2
 CFLAGS += -D__EXPORTED_HEADERS__ -I$(srctree)/include/uapi -I$(srctree)/include

@@ -67,9 +68,10 @@ clean: bpftool_clean
 	       $(OUTPUT)bpf_asm $(OUTPUT)bpf_exp.yacc.* $(OUTPUT)bpf_exp.lex.*
 
 install: bpftool_install
-	install $(OUTPUT)bpf_jit_disasm $(prefix)/bin/bpf_jit_disasm
-	install $(OUTPUT)bpf_dbg $(prefix)/bin/bpf_dbg
-	install $(OUTPUT)bpf_asm $(prefix)/bin/bpf_asm
+	$(INSTALL) -m 0755 -d $(DESTDIR)$(prefix)/bin
+	$(INSTALL) $(OUTPUT)bpf_jit_disasm $(DESTDIR)$(prefix)/bin/bpf_jit_disasm
+	$(INSTALL) $(OUTPUT)bpf_dbg $(DESTDIR)$(prefix)/bin/bpf_dbg
+	$(INSTALL) $(OUTPUT)bpf_asm $(DESTDIR)$(prefix)/bin/bpf_asm
 
 bpftool:
 	$(MAKE) -C bpftool

Signature · 1 line

1.8.3.1

Jiri Benc

author

2018-03-08 22:00 UTC #6 lore raw

PATCH 4/7 tools: bpf: make install should build first

In reply to Jiri Benc

Make the 'install' target depend on the 'all' target to build the binaries first.

Signed-off-by Jiri Benc <[email protected]>

---
 tools/bpf/Makefile | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

tools/bpf/Makefile +4 -2

index c42ca24a072d..e8d9e4125bf3 100644
@@ -50,7 +50,9 @@ $(OUTPUT)%.lex.c: $(srctree)/tools/bpf/%.l
 $(OUTPUT)%.o: $(srctree)/tools/bpf/%.c
 	$(COMPILE.c) -o $@ $<
 
-all: $(OUTPUT)bpf_jit_disasm $(OUTPUT)bpf_dbg $(OUTPUT)bpf_asm bpftool
+PROGS = $(OUTPUT)bpf_jit_disasm $(OUTPUT)bpf_dbg $(OUTPUT)bpf_asm
+
+all: $(PROGS) bpftool
 
 $(OUTPUT)bpf_jit_disasm: CFLAGS += -DPACKAGE='bpf_jit_disasm'
 $(OUTPUT)bpf_jit_disasm: LDLIBS = -lopcodes -lbfd -ldl

@@ -67,7 +69,7 @@ clean: bpftool_clean
 	rm -rf $(OUTPUT)*.o $(OUTPUT)bpf_jit_disasm $(OUTPUT)bpf_dbg \
 	       $(OUTPUT)bpf_asm $(OUTPUT)bpf_exp.yacc.* $(OUTPUT)bpf_exp.lex.*
 
-install: bpftool_install
+install: $(PROGS) bpftool_install
 	$(INSTALL) -m 0755 -d $(DESTDIR)$(prefix)/bin
 	$(INSTALL) $(OUTPUT)bpf_jit_disasm $(DESTDIR)$(prefix)/bin/bpf_jit_disasm
 	$(INSTALL) $(OUTPUT)bpf_dbg $(DESTDIR)$(prefix)/bin/bpf_dbg

Signature · 1 line

1.8.3.1

Jiri Benc

author

2018-03-08 22:00 UTC #7 lore raw

PATCH 5/7 tools: bpf: call descend in Makefile

In reply to Jiri Benc

Use the descend macro to properly propagate $(subdir) to bpftool.

Signed-off-by Jiri Benc <[email protected]>

---
 tools/bpf/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

tools/bpf/Makefile +3 -3

index e8d9e4125bf3..daca0a4277d1 100644
@@ -76,12 +76,12 @@ install: $(PROGS) bpftool_install
 	$(INSTALL) $(OUTPUT)bpf_asm $(DESTDIR)$(prefix)/bin/bpf_asm
 
 bpftool:
-	$(MAKE) -C bpftool
+	$(call descend,bpftool)
 
 bpftool_install:
-	$(MAKE) -C bpftool install
+	$(call descend,bpftool,install)
 
 bpftool_clean:
-	$(MAKE) -C bpftool clean
+	$(call descend,bpftool,clean)
 
 .PHONY: bpftool FORCE

Signature · 1 line

1.8.3.1

Jiri Benc

author

2018-03-08 22:00 UTC #8 lore raw

PATCH 6/7 tools: bpf: respect quiet/verbose build

In reply to Jiri Benc

Default to quiet build, with V=1 enabling verbose build as is usual.

Signed-off-by Jiri Benc <[email protected]>

---
 tools/bpf/Makefile | 38 +++++++++++++++++++++++++++-----------
 1 file changed, 27 insertions(+), 11 deletions(-)

tools/bpf/Makefile +27 ~~-11~~

index daca0a4277d1..757ea22c428a 100644
@@ -17,6 +17,12 @@ srctree := $(patsubst %/,%,$(dir $(CURDIR)))
 srctree := $(patsubst %/,%,$(dir $(srctree)))
 endif
 
+ifeq ($(V),1)
+  Q =
+else
+  Q = @
+endif
+
 FEATURE_USER = .bpf
 FEATURE_TESTS = libbfd disassembler-four-args
 FEATURE_DISPLAY = libbfd disassembler-four-args

@@ -42,38 +48,48 @@ CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
 endif
 
 $(OUTPUT)%.yacc.c: $(srctree)/tools/bpf/%.y
-	$(YACC) -o $@ -d $<
+	$(QUIET_BISON)$(YACC) -o $@ -d $<
 
 $(OUTPUT)%.lex.c: $(srctree)/tools/bpf/%.l
-	$(LEX) -o $@ $<
+	$(QUIET_FLEX)$(LEX) -o $@ $<
 
 $(OUTPUT)%.o: $(srctree)/tools/bpf/%.c
-	$(COMPILE.c) -o $@ $<
+	$(QUIET_CC)$(COMPILE.c) -o $@ $<
+
+$(OUTPUT)%.yacc.o: $(OUTPUT)%.yacc.c
+	$(QUIET_CC)$(COMPILE.c) -o $@ $<
+$(OUTPUT)%.lex.o: $(OUTPUT)%.lex.c
+	$(QUIET_CC)$(COMPILE.c) -o $@ $<
 
 PROGS = $(OUTPUT)bpf_jit_disasm $(OUTPUT)bpf_dbg $(OUTPUT)bpf_asm
 
 all: $(PROGS) bpftool
 
 $(OUTPUT)bpf_jit_disasm: CFLAGS += -DPACKAGE='bpf_jit_disasm'
-$(OUTPUT)bpf_jit_disasm: LDLIBS = -lopcodes -lbfd -ldl
 $(OUTPUT)bpf_jit_disasm: $(OUTPUT)bpf_jit_disasm.o
+	$(QUIET_LINK)$(CC) $(CFLAGS) -o $@ $^ -lopcodes -lbfd -ldl
 
-$(OUTPUT)bpf_dbg: LDLIBS = -lreadline
 $(OUTPUT)bpf_dbg: $(OUTPUT)bpf_dbg.o
+	$(QUIET_LINK)$(CC) $(CFLAGS) -o $@ $^ -lreadline
 
-$(OUTPUT)bpf_asm: LDLIBS =
 $(OUTPUT)bpf_asm: $(OUTPUT)bpf_asm.o $(OUTPUT)bpf_exp.yacc.o $(OUTPUT)bpf_exp.lex.o
+	$(QUIET_LINK)$(CC) $(CFLAGS) -o $@ $^
+
 $(OUTPUT)bpf_exp.lex.o: $(OUTPUT)bpf_exp.yacc.c
 
 clean: bpftool_clean
-	rm -rf $(OUTPUT)*.o $(OUTPUT)bpf_jit_disasm $(OUTPUT)bpf_dbg \
+	$(call QUIET_CLEAN, bpf-progs)
+	$(Q)rm -rf $(OUTPUT)*.o $(OUTPUT)bpf_jit_disasm $(OUTPUT)bpf_dbg \
 	       $(OUTPUT)bpf_asm $(OUTPUT)bpf_exp.yacc.* $(OUTPUT)bpf_exp.lex.*
 
 install: $(PROGS) bpftool_install
-	$(INSTALL) -m 0755 -d $(DESTDIR)$(prefix)/bin
-	$(INSTALL) $(OUTPUT)bpf_jit_disasm $(DESTDIR)$(prefix)/bin/bpf_jit_disasm
-	$(INSTALL) $(OUTPUT)bpf_dbg $(DESTDIR)$(prefix)/bin/bpf_dbg
-	$(INSTALL) $(OUTPUT)bpf_asm $(DESTDIR)$(prefix)/bin/bpf_asm
+	$(call QUIET_INSTALL, bpf_jit_disasm)
+	$(Q)$(INSTALL) -m 0755 -d $(DESTDIR)$(prefix)/bin
+	$(Q)$(INSTALL) $(OUTPUT)bpf_jit_disasm $(DESTDIR)$(prefix)/bin/bpf_jit_disasm
+	$(call QUIET_INSTALL, bpf_dbg)
+	$(Q)$(INSTALL) $(OUTPUT)bpf_dbg $(DESTDIR)$(prefix)/bin/bpf_dbg
+	$(call QUIET_INSTALL, bpf_asm)
+	$(Q)$(INSTALL) $(OUTPUT)bpf_asm $(DESTDIR)$(prefix)/bin/bpf_asm
 
 bpftool:
 	$(call descend,bpftool)

Signature · 1 line

1.8.3.1

Jiri Benc

author

2018-03-08 22:01 UTC #9 lore raw

PATCH 7/7 tools: bpf: silence make by not deleting intermediate file

In reply to Jiri Benc

Even in quiet mode, make finishes with

rm tools/bpf/bpf_exp.lex.c

That's because it considers the file to be intermediate. Silence that by mentioning the lex.c file instead of the lex.o file; the dependency still stays.

Signed-off-by Jiri Benc <[email protected]>

---
 tools/bpf/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tools/bpf/Makefile +1 -1

index 757ea22c428a..c07b4e718eeb 100644
@@ -75,7 +75,7 @@ $(OUTPUT)bpf_dbg: $(OUTPUT)bpf_dbg.o
 $(OUTPUT)bpf_asm: $(OUTPUT)bpf_asm.o $(OUTPUT)bpf_exp.yacc.o $(OUTPUT)bpf_exp.lex.o
 	$(QUIET_LINK)$(CC) $(CFLAGS) -o $@ $^
 
-$(OUTPUT)bpf_exp.lex.o: $(OUTPUT)bpf_exp.yacc.c
+$(OUTPUT)bpf_exp.lex.c: $(OUTPUT)bpf_exp.yacc.c
 
 clean: bpftool_clean
 	$(call QUIET_CLEAN, bpf-progs)

Signature · 1 line

1.8.3.1

Alexei Starovoitov

2018-03-09 05:11 UTC #10 lore raw

In reply to Jiri Benc

Quoted 8 lines · Jiri Benc

Currently, 'make bpf' in the tools/ directory does not provide the standard quiet output except for bpftool (which is however listed with a wrong directory). Worse, it does not respect the build output directory.
The 'make bpf_install' does not work as one would expect, either. It installs unconditionally to /usr/bin without respecting DESTDIR and prefix.
This patchset improves that behavior.

Jakub,
please review this set.

Thanks!

Jakub Kicinski

Reviewed-by

2018-03-09 07:36 UTC #11 lore raw

Quoted 8 lines · Alexei Starovoitov

Quoted 7 lines · Jiri Benc
Currently, 'make bpf' in the tools/ directory does not provide the standard quiet output except for bpftool (which is however listed with a wrong directory). Worse, it does not respect the build output directory.
The 'make bpf_install' does not work as one would expect, either. It installs unconditionally to /usr/bin without respecting DESTDIR and prefix.

Quoted 6 lines

Quoted 1 line
This patchset improves that behavior.
Jakub,
please review this set.
Thanks!

I stared at it for a while, and it looks good!

Reviewed-by Jakub Kicinski <[email protected]>

In addition I found a GCC 7 warning in bpftool, .PHONY lacks some targets and we don't clean up after feature detection. I'll send patches for that unless someone beats me to it.

Daniel Borkmann

maintainer

2018-03-09 09:29 UTC #12 lore raw

In reply to Jiri Benc

Quoted 18 lines · Jiri Benc

Currently, 'make bpf' in the tools/ directory does not provide the standard quiet output except for bpftool (which is however listed with a wrong directory). Worse, it does not respect the build output directory.
The 'make bpf_install' does not work as one would expect, either. It installs unconditionally to /usr/bin without respecting DESTDIR and prefix.
This patchset improves that behavior.
Jiri Benc (7):
tools: bpftool: silence 'missing initializer' warnings
tools: bpf: respect output directory during build
tools: bpf: consistent make bpf_install
tools: bpf: make install should build first
tools: bpf: call descend in Makefile
tools: bpf: respect quiet/verbose build
tools: bpf: silence make by not deleting intermediate file

Quoted 3 lines

 tools/bpf/Makefile         | 74 +++++++++++++++++++++++++++++++---------------
 tools/bpf/bpftool/Makefile |  2 +-
 2 files changed, 51 insertions(+), 25 deletions(-)

Looks good to me as well, applied to bpf-next, thanks Jiri!