[PATCH v3 bpf-next 00/14] samples: bpf: improve/fix cross-compilation

[PATCH v3 bpf-next 00/14] samples: bpf: improve/fix cross-compilation

[Ivan Khoronzhuk]

This series contains mainly fixes/improvements for cross-compilation
but not only, tested for arm, arm64, but intended for any arch.
Also verified on native build (not cross compilation) for x86_64
and arm.

Initial RFC link:
https://lkml.org/lkml/2019/8/29/1665

Prev. version:
https://lkml.org/lkml/2019/9/10/331

Besides the patches given here, the RFC also contains couple patches
related to llvm clang
  arm: include: asm: swab: mask rev16 instruction for clang
  arm: include: asm: unified: mask .syntax unified for clang
They are necessarily to verify arm build.

The change touches not only cross-compilation and can have impact on
other archs and build environments, so might be good idea to verify
it in order to add appropriate changes, some warn options could be
tuned also.

All is tested on x86-64 with clang installed (has to be built containing
targets for arm, arm64..., see llc --version, usually it's present already)

Instructions to test native on x86_64
=================================================
Native build on x86_64 is done in usual way and shouldn't have difference
except HOSTCC is now printed as CC wile building the samples.

Instructions to test cross compilation on arm64
=================================================
#Toolchain used for test:
gcc version 8.3.0
(GNU Toolchain for the A-profile Architecture 8.3-2019.03 (arm-rel-8.36))

# Get some arm64 FS, containing at least libelf
I've used sdk for TI am65x got here:
http://downloads.ti.com/processor-sdk-linux/esd/AM65X/latest/exports/\
ti-processor-sdk-linux-am65xx-evm-06.00.00.07-Linux-x86-Install.bin

# Install this binary to some dir, say "sdk".
# Configure kernel (use defconfig as no matter), but clean everything
# before.
make ARCH=arm64 -C tools/ clean
make ARCH=arm64 -C samples/bpf clean
make ARCH=arm64 clean
make ARCH=arm64 defconfig

# The kernel version used in sdk doesn't correspond to checked one,
# but for this verification only headers need to be syched,
# so install them:
make ARCH=arm64 headers_install

# or on SDK if need keep them in sync (not necessarily to verify):

make ARCH=arm64 INSTALL_HDR_PATH=/../sdk/\
ti-processor-sdk-linux-am65xx-evm-06.00.00.07/linux-devkit/sysroots/\
aarch64-linux/usr headers_install

# Build samples
make samples/bpf/ ARCH=arm64 CROSS_COMPILE="aarch64-linux-gnu-"\
SYSROOT="/../sdk/ti-processor-sdk-linux-am65xx-evm-06.00.00.07/\
linux-devkit/sysroots/aarch64-linux"

Instructions to test cross compilation on arm
=================================================
#Toolchains used for test:
arm-linux-gnueabihf-gcc (Linaro GCC 7.2-2017.11) 7.2.1 20171011
or
arm-linux-gnueabihf-gcc
(GNU Toolchain for the A-profile Architecture 8.3-2019.03 \
(arm-rel-8.36)) 8.3.0

# Get some FS, I've used sdk for TI am52xx got here:
http://downloads.ti.com/processor-sdk-linux/esd/AM57X/05_03_00_07/exports/\
ti-processor-sdk-linux-am57xx-evm-05.03.00.07-Linux-x86-Install.bin

# Install this binary to some dir, say "sdk".
# Configure kernel, but clean everything before.
make ARCH=arm -C tools/ clean
make ARCH=arm -C samples/bpf clean
make ARCH=arm clean
make ARCH=arm omap2plus_defconfig

# The kernel version used in sdk doesn't correspond to checked one, but
headers only should be synched, so install them:

make ARCH=arm64 headers_install

# or on SDK if need keep them in sync (not necessarily):

make ARCH=arm INSTALL_HDR_PATH=/../sdk/\
ti-processor-sdk-linux-am57xx-evm-05.03.00.07/linux-devkit/sysroots/\
armv7ahf-neon-linux-gnueabi/usr headers_install

# Build samples
make samples/bpf/ ARCH=arm CROSS_COMPILE="arm-linux-gnueabihf-"\
SYSROOT="/../sdk/ti-processor-sdk-linux-am57xx-evm-05.03\
.00.07/linux-devkit/sysroots/armv7ahf-neon-linux-gnueabi"


Based on bpf-next/master

v3..v2:
- renamed makefile.progs to makeifle.target, as more appropriate
- left only __LINUX_ARM_ARCH__ for D options for arm
- for host build - left options from KBUILD_HOST for compatibility reasons
- split patch adding c/cxx/ld flags to libbpf by modules
- moved readme change to separate patch
- added patch setting options for cross-compile
- fixed issue with option error for syscall_nrs.S,
  avoiding overlap for ccflags-y.

v2..v1:
- restructured patches order
- split "samples: bpf: Makefile: base progs build on Makefile.progs"
  to make change more readable. It added couple nice extra patches.
- removed redundant patch:
  "samples: bpf: Makefile: remove target for native build"
- added fix:
  "samples: bpf: makefile: fix cookie_uid_helper_example obj build"
- limited -D option filter only for arm
- improved comments
- added couple instructions to verify cross compilation for arm and
  arm64 arches based on TI am57xx and am65xx sdks.
- corrected include a little order

Ivan Khoronzhuk (14):
  samples: bpf: makefile: fix HDR_PROBE "echo"
  samples: bpf: makefile: fix cookie_uid_helper_example obj build
  samples: bpf: makefile: use --target from cross-compile
  samples: bpf: use own EXTRA_CFLAGS for clang commands
  samples: bpf: makefile: use __LINUX_ARM_ARCH__ selector for arm
  samples: bpf: makefile: drop unnecessarily inclusion for bpf_load
  samples: bpf: add makefile.target for separate CC target build
  samples: bpf: makefile: base target programs rules on Makefile.target
  samples: bpf: makefile: use own flags but not host when cross compile
  samples: bpf: makefile: use target CC environment for HDR_PROBE
  libbpf: makefile: add C/CXX/LDFLAGS to libbpf.so and test_libpf
    targets
  samples: bpf: makefile: provide C/CXX/LD flags to libbpf
  samples: bpf: makefile: add sysroot support
  samples: bpf: README: add preparation steps and sysroot info

 samples/bpf/Makefile        | 179 +++++++++++++++++++++---------------
 samples/bpf/Makefile.target |  75 +++++++++++++++
 samples/bpf/README.rst      |  41 ++++++++-
 tools/lib/bpf/Makefile      |  11 ++-
 4 files changed, 225 insertions(+), 81 deletions(-)
 create mode 100644 samples/bpf/Makefile.target

-- 
2.17.1

[PATCH v3 bpf-next 01/14] samples: bpf: makefile: fix HDR_PROBE "echo"

[Ivan Khoronzhuk]

echo should be replaced with echo -e to handle '\n' correctly, but
instead, replace it with printf as some systems can't handle echo -e.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
 samples/bpf/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 1d9be26b4edd..f50ca852c2a8 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -201,7 +201,7 @@ endif
 
 # Don't evaluate probes and warnings if we need to run make recursively
 ifneq ($(src),)
-HDR_PROBE := $(shell echo "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \
+HDR_PROBE := $(shell printf "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \
 	$(HOSTCC) $(KBUILD_HOSTCFLAGS) -x c - -o /dev/null 2>/dev/null && \
 	echo okay)
 
-- 
2.17.1

[PATCH v3 bpf-next 02/14] samples: bpf: makefile: fix cookie_uid_helper_example obj build

[Ivan Khoronzhuk]

Don't list userspace "cookie_uid_helper_example" object in list for
bpf objects.

'always' target is used for listing bpf programs, but
'cookie_uid_helper_example.o' is a user space ELF file, and covered
by rule `per_socket_stats_example`, so shouldn't be in 'always'.
Let us remove `always += cookie_uid_helper_example.o`, which avoids
breaking cross compilation due to mismatched includes.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
 samples/bpf/Makefile | 1 -
 1 file changed, 1 deletion(-)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index f50ca852c2a8..43dee90dffa4 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -145,7 +145,6 @@ always += sampleip_kern.o
 always += lwt_len_hist_kern.o
 always += xdp_tx_iptunnel_kern.o
 always += test_map_in_map_kern.o
-always += cookie_uid_helper_example.o
 always += tcp_synrto_kern.o
 always += tcp_rwnd_kern.o
 always += tcp_bufs_kern.o
-- 
2.17.1

[PATCH v3 bpf-next 03/14] samples: bpf: makefile: use --target from cross-compile

[Ivan Khoronzhuk]

For cross compiling the target triple can be inherited from
cross-compile prefix as it's done in CLANG_FLAGS from kernel makefile.
So copy-paste this decision from kernel Makefile.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
 samples/bpf/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 43dee90dffa4..b59e77e2250e 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -195,7 +195,7 @@ BTF_PAHOLE ?= pahole
 # Detect that we're cross compiling and use the cross compiler
 ifdef CROSS_COMPILE
 HOSTCC = $(CROSS_COMPILE)gcc
-CLANG_ARCH_ARGS = -target $(ARCH)
+CLANG_ARCH_ARGS = --target=$(notdir $(CROSS_COMPILE:%-=%))
 endif
 
 # Don't evaluate probes and warnings if we need to run make recursively
-- 
2.17.1

[PATCH v3 bpf-next 04/14] samples: bpf: use own EXTRA_CFLAGS for clang commands

[Ivan Khoronzhuk]

It can overlap with CFLAGS used for libraries built with gcc if
not now then in next patches. Correct it here for simplicity.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
 samples/bpf/Makefile | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index b59e77e2250e..8ecc5d0c2d5b 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -218,10 +218,10 @@ BTF_LLVM_PROBE := $(shell echo "int main() { return 0; }" | \
 			  /bin/rm -f ./llvm_btf_verify.o)
 
 ifneq ($(BTF_LLVM_PROBE),)
-	EXTRA_CFLAGS += -g
+	CLANG_EXTRA_CFLAGS += -g
 else
 ifneq ($(and $(BTF_LLC_PROBE),$(BTF_PAHOLE_PROBE),$(BTF_OBJCOPY_PROBE)),)
-	EXTRA_CFLAGS += -g
+	CLANG_EXTRA_CFLAGS += -g
 	LLC_FLAGS += -mattr=dwarfris
 	DWARF2BTF = y
 endif
@@ -280,8 +280,8 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
 # useless for BPF samples.
 $(obj)/%.o: $(src)/%.c
 	@echo "  CLANG-bpf " $@
-	$(Q)$(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) -I$(obj) \
-		-I$(srctree)/tools/testing/selftests/bpf/ \
+	$(Q)$(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(CLANG_EXTRA_CFLAGS) \
+		-I$(obj) -I$(srctree)/tools/testing/selftests/bpf/ \
 		-D__KERNEL__ -D__BPF_TRACING__ -Wno-unused-value -Wno-pointer-sign \
 		-D__TARGET_ARCH_$(SRCARCH) -Wno-compare-distinct-pointer-types \
 		-Wno-gnu-variable-sized-type-not-at-end \
-- 
2.17.1

[PATCH v3 bpf-next 05/14] samples: bpf: makefile: use __LINUX_ARM_ARCH__ selector for arm

[Ivan Khoronzhuk]

For arm, -D__LINUX_ARM_ARCH__=X is min version used as instruction
set selector and is absolutely required while parsing some parts of
headers. It's present in KBUILD_CFLAGS but not in autoconf.h, so let's
retrieve it from and add to programs cflags. In another case errors
like "SMP is not supported" for armv7 and bunch of other errors are
issued resulting to incorrect final object.
---
 samples/bpf/Makefile | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 8ecc5d0c2d5b..d3c8db3df560 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -185,6 +185,16 @@ HOSTLDLIBS_map_perf_test	+= -lrt
 HOSTLDLIBS_test_overhead	+= -lrt
 HOSTLDLIBS_xdpsock		+= -pthread
 
+ifeq ($(ARCH), arm)
+# Strip all except -D__LINUX_ARM_ARCH__ option needed to handle linux
+# headers when arm instruction set identification is requested.
+ARM_ARCH_SELECTOR = $(shell echo "$(KBUILD_CFLAGS) " | \
+		    sed 's/[[:blank:]]/\n/g' | sed '/^-D__LINUX_ARM_ARCH__/!d')
+
+CLANG_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR)
+KBUILD_HOSTCFLAGS := $(ARM_ARCH_SELECTOR)
+endif
+
 # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline:
 #  make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
 LLC ?= llc
-- 
2.17.1

[PATCH v3 bpf-next 06/14] samples: bpf: makefile: drop unnecessarily inclusion for bpf_load

[Ivan Khoronzhuk]

Drop inclusion for bpf_load -I$(objtree)/usr/include as it is
included for all objects anyway, with above line:
KBUILD_HOSTCFLAGS += -I$(objtree)/usr/include

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
 samples/bpf/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index d3c8db3df560..9d923546e087 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -176,7 +176,7 @@ KBUILD_HOSTCFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
 KBUILD_HOSTCFLAGS += -I$(srctree)/tools/lib/ -I$(srctree)/tools/include
 KBUILD_HOSTCFLAGS += -I$(srctree)/tools/perf
 
-HOSTCFLAGS_bpf_load.o += -I$(objtree)/usr/include -Wno-unused-variable
+HOSTCFLAGS_bpf_load.o += -Wno-unused-variable
 
 KBUILD_HOSTLDLIBS		+= $(LIBBPF) -lelf
 HOSTLDLIBS_tracex4		+= -lrt
-- 
2.17.1

[PATCH v3 bpf-next 07/14] samples: bpf: add makefile.target for separate CC target build

[Ivan Khoronzhuk]

The makefile.target is added only and will be used in
sample/bpf/Makefile later in order to switch cross-compiling on CC
from HOSTCC environment.

The HOSTCC is supposed to build binaries and tools running on the host
afterwards, in order to simplify build or so, like "fixdep" or else.
In case of cross compiling "fixdep" is executed on host when the rest
samples should run on target arch. In order to build binaries for
target arch with CC and tools running on host with HOSTCC, lets add
Makefile.target for simplicity, having definition and routines similar
to ones, used in script/Makefile.host. This allows later add
cross-compilation to samples/bpf with minimum changes.

The tprog stands for target programs built with CC.

Makefile.target contains only stuff needed for samples/bpf, potentially
can be reused later and now needed only for unblocking tricky
samples/bpf cross compilation.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
 samples/bpf/Makefile.target | 75 +++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)
 create mode 100644 samples/bpf/Makefile.target

diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
new file mode 100644
index 000000000000..fb6de63f7d2f
--- /dev/null
+++ b/samples/bpf/Makefile.target
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: GPL-2.0
+# ==========================================================================
+# Building binaries on the host system
+# Binaries are not used during the compilation of the kernel, and intendent
+# to be build for target board, target board can be host ofc. Added to build
+# binaries to run not on host system.
+#
+# Sample syntax (see Documentation/kbuild/makefiles.rst for reference)
+# tprogs-y := xsk_example
+# Will compile xdpsock_example.c and create an executable named xsk_example
+#
+# tprogs-y    := xdpsock
+# xdpsock-objs := xdpsock_1.o xdpsock_2.o
+# Will compile xdpsock_1.c and xdpsock_2.c, and then link the executable
+# xdpsock, based on xdpsock_1.o and xdpsock_2.o
+#
+# Inherited from scripts/Makefile.host
+#
+__tprogs := $(sort $(tprogs-y))
+
+# C code
+# Executables compiled from a single .c file
+tprog-csingle	:= $(foreach m,$(__tprogs), \
+			$(if $($(m)-objs),,$(m)))
+
+# C executables linked based on several .o files
+tprog-cmulti	:= $(foreach m,$(__tprogs),\
+			$(if $($(m)-objs),$(m)))
+
+# Object (.o) files compiled from .c files
+tprog-cobjs	:= $(sort $(foreach m,$(__tprogs),$($(m)-objs)))
+
+tprog-csingle	:= $(addprefix $(obj)/,$(tprog-csingle))
+tprog-cmulti	:= $(addprefix $(obj)/,$(tprog-cmulti))
+tprog-cobjs	:= $(addprefix $(obj)/,$(tprog-cobjs))
+
+#####
+# Handle options to gcc. Support building with separate output directory
+
+_tprogc_flags   = $(TPROGS_CFLAGS) \
+                 $(TPROGCFLAGS_$(basetarget).o)
+
+# $(objtree)/$(obj) for including generated headers from checkin source files
+ifeq ($(KBUILD_EXTMOD),)
+ifdef building_out_of_srctree
+_tprogc_flags   += -I $(objtree)/$(obj)
+endif
+endif
+
+tprogc_flags    = -Wp,-MD,$(depfile) $(_tprogc_flags)
+
+# Create executable from a single .c file
+# tprog-csingle -> Executable
+quiet_cmd_tprog-csingle 	= CC  $@
+      cmd_tprog-csingle	= $(CC) $(tprogc_flags) $(TPROGS_LDFLAGS) -o $@ $< \
+		$(TPROGS_LDLIBS) $(TPROGLDLIBS_$(@F))
+$(tprog-csingle): $(obj)/%: $(src)/%.c FORCE
+	$(call if_changed_dep,tprog-csingle)
+
+# Link an executable based on list of .o files, all plain c
+# tprog-cmulti -> executable
+quiet_cmd_tprog-cmulti	= LD  $@
+      cmd_tprog-cmulti	= $(CC) $(tprogc_flags) $(TPROGS_LDFLAGS) -o $@ \
+			  $(addprefix $(obj)/,$($(@F)-objs)) \
+			  $(TPROGS_LDLIBS) $(TPROGLDLIBS_$(@F))
+$(tprog-cmulti): $(tprog-cobjs) FORCE
+	$(call if_changed,tprog-cmulti)
+$(call multi_depend, $(tprog-cmulti), , -objs)
+
+# Create .o file from a single .c file
+# tprog-cobjs -> .o
+quiet_cmd_tprog-cobjs	= CC  $@
+      cmd_tprog-cobjs	= $(CC) $(tprogc_flags) -c -o $@ $<
+$(tprog-cobjs): $(obj)/%.o: $(src)/%.c FORCE
+	$(call if_changed_dep,tprog-cobjs)
-- 
2.17.1

[PATCH v3 bpf-next 08/14] samples: bpf: makefile: base target programs rules on Makefile.target

[Ivan Khoronzhuk]

The main reason for that - HOSTCC and CC have different aims.
HOSTCC is used to build programs running on host, that can
cross-comple target programs with CC. It was tested for arm and arm64
cross compilation, based on linaro toolchain, but should work for
others.

So, in order to split cross compilation (CC) with host build (HOSTCC),
lets base samples on Makefile.target. It allows to cross-compile
samples/bpf programs with CC while auxialry tools running on host
built with HOSTCC.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
 samples/bpf/Makefile | 135 ++++++++++++++++++++++---------------------
 1 file changed, 69 insertions(+), 66 deletions(-)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 9d923546e087..1579cc16a1c2 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -4,55 +4,53 @@ BPF_SAMPLES_PATH ?= $(abspath $(srctree)/$(src))
 TOOLS_PATH := $(BPF_SAMPLES_PATH)/../../tools
 
 # List of programs to build
-hostprogs-y := test_lru_dist
-hostprogs-y += sock_example
-hostprogs-y += fds_example
-hostprogs-y += sockex1
-hostprogs-y += sockex2
-hostprogs-y += sockex3
-hostprogs-y += tracex1
-hostprogs-y += tracex2
-hostprogs-y += tracex3
-hostprogs-y += tracex4
-hostprogs-y += tracex5
-hostprogs-y += tracex6
-hostprogs-y += tracex7
-hostprogs-y += test_probe_write_user
-hostprogs-y += trace_output
-hostprogs-y += lathist
-hostprogs-y += offwaketime
-hostprogs-y += spintest
-hostprogs-y += map_perf_test
-hostprogs-y += test_overhead
-hostprogs-y += test_cgrp2_array_pin
-hostprogs-y += test_cgrp2_attach
-hostprogs-y += test_cgrp2_sock
-hostprogs-y += test_cgrp2_sock2
-hostprogs-y += xdp1
-hostprogs-y += xdp2
-hostprogs-y += xdp_router_ipv4
-hostprogs-y += test_current_task_under_cgroup
-hostprogs-y += trace_event
-hostprogs-y += sampleip
-hostprogs-y += tc_l2_redirect
-hostprogs-y += lwt_len_hist
-hostprogs-y += xdp_tx_iptunnel
-hostprogs-y += test_map_in_map
-hostprogs-y += per_socket_stats_example
-hostprogs-y += xdp_redirect
-hostprogs-y += xdp_redirect_map
-hostprogs-y += xdp_redirect_cpu
-hostprogs-y += xdp_monitor
-hostprogs-y += xdp_rxq_info
-hostprogs-y += syscall_tp
-hostprogs-y += cpustat
-hostprogs-y += xdp_adjust_tail
-hostprogs-y += xdpsock
-hostprogs-y += xdp_fwd
-hostprogs-y += task_fd_query
-hostprogs-y += xdp_sample_pkts
-hostprogs-y += ibumad
-hostprogs-y += hbm
+tprogs-y := test_lru_dist
+tprogs-y += sock_example
+tprogs-y += fds_example
+tprogs-y += sockex1
+tprogs-y += sockex2
+tprogs-y += sockex3
+tprogs-y += tracex1
+tprogs-y += tracex2
+tprogs-y += tracex3
+tprogs-y += tracex4
+tprogs-y += tracex5
+tprogs-y += tracex6
+tprogs-y += tracex7
+tprogs-y += test_probe_write_user
+tprogs-y += trace_output
+tprogs-y += lathist
+tprogs-y += offwaketime
+tprogs-y += spintest
+tprogs-y += map_perf_test
+tprogs-y += test_overhead
+tprogs-y += test_cgrp2_array_pin
+tprogs-y += test_cgrp2_attach
+tprogs-y += test_cgrp2_sock
+tprogs-y += test_cgrp2_sock2
+tprogs-y += xdp1
+tprogs-y += xdp2
+tprogs-y += xdp_router_ipv4
+tprogs-y += test_current_task_under_cgroup
+tprogs-y += trace_event
+tprogs-y += sampleip
+tprogs-y += tc_l2_redirect
+tprogs-y += lwt_len_hist
+tprogs-y += xdp_tx_iptunnel
+tprogs-y += test_map_in_map
+tprogs-y += xdp_redirect_map
+tprogs-y += xdp_redirect_cpu
+tprogs-y += xdp_monitor
+tprogs-y += xdp_rxq_info
+tprogs-y += syscall_tp
+tprogs-y += cpustat
+tprogs-y += xdp_adjust_tail
+tprogs-y += xdpsock
+tprogs-y += xdp_fwd
+tprogs-y += task_fd_query
+tprogs-y += xdp_sample_pkts
+tprogs-y += ibumad
+tprogs-y += hbm
 
 # Libbpf dependencies
 LIBBPF = $(TOOLS_PATH)/lib/bpf/libbpf.a
@@ -111,7 +109,7 @@ ibumad-objs := bpf_load.o ibumad_user.o $(TRACE_HELPERS)
 hbm-objs := bpf_load.o hbm.o $(CGROUP_HELPERS)
 
 # Tell kbuild to always build the programs
-always := $(hostprogs-y)
+always := $(tprogs-y)
 always += sockex1_kern.o
 always += sockex2_kern.o
 always += sockex3_kern.o
@@ -170,21 +168,6 @@ always += ibumad_kern.o
 always += hbm_out_kern.o
 always += hbm_edt_kern.o
 
-KBUILD_HOSTCFLAGS += -I$(objtree)/usr/include
-KBUILD_HOSTCFLAGS += -I$(srctree)/tools/lib/bpf/
-KBUILD_HOSTCFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
-KBUILD_HOSTCFLAGS += -I$(srctree)/tools/lib/ -I$(srctree)/tools/include
-KBUILD_HOSTCFLAGS += -I$(srctree)/tools/perf
-
-HOSTCFLAGS_bpf_load.o += -Wno-unused-variable
-
-KBUILD_HOSTLDLIBS		+= $(LIBBPF) -lelf
-HOSTLDLIBS_tracex4		+= -lrt
-HOSTLDLIBS_trace_output	+= -lrt
-HOSTLDLIBS_map_perf_test	+= -lrt
-HOSTLDLIBS_test_overhead	+= -lrt
-HOSTLDLIBS_xdpsock		+= -pthread
-
 ifeq ($(ARCH), arm)
 # Strip all except -D__LINUX_ARM_ARCH__ option needed to handle linux
 # headers when arm instruction set identification is requested.
@@ -192,9 +175,27 @@ ARM_ARCH_SELECTOR = $(shell echo "$(KBUILD_CFLAGS) " | \
 		    sed 's/[[:blank:]]/\n/g' | sed '/^-D__LINUX_ARM_ARCH__/!d')
 
 CLANG_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR)
-KBUILD_HOSTCFLAGS := $(ARM_ARCH_SELECTOR)
+TPROGS_CFLAGS += $(ARM_ARCH_SELECTOR)
 endif
 
+TPROGS_LDLIBS := $(KBUILD_HOSTLDLIBS)
+TPROGS_CFLAGS += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS)
+TPROGS_CFLAGS += -I$(objtree)/usr/include
+TPROGS_CFLAGS += -I$(srctree)/tools/lib/bpf/
+TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
+TPROGS_CFLAGS += -I$(srctree)/tools/lib/
+TPROGS_CFLAGS += -I$(srctree)/tools/include
+TPROGS_CFLAGS += -I$(srctree)/tools/perf
+
+TPROGCFLAGS_bpf_load.o += -Wno-unused-variable
+
+TPROGS_LDLIBS			+= $(LIBBPF) -lelf
+TPROGLDLIBS_tracex4		+= -lrt
+TPROGLDLIBS_trace_output	+= -lrt
+TPROGLDLIBS_map_perf_test	+= -lrt
+TPROGLDLIBS_test_overhead	+= -lrt
+TPROGLDLIBS_xdpsock		+= -pthread
+
 # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline:
 #  make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
 LLC ?= llc
@@ -285,6 +286,8 @@ $(obj)/hbm_out_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
 $(obj)/hbm.o: $(src)/hbm.h
 $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
 
+-include $(BPF_SAMPLES_PATH)/Makefile.target
+
 # asm/sysreg.h - inline assembly used by it is incompatible with llvm.
 # But, there is no easy way to fix it, so just exclude it since it is
 # useless for BPF samples.
-- 
2.17.1

[PATCH v3 bpf-next 09/14] samples: bpf: makefile: use own flags but not host when cross compile

[Ivan Khoronzhuk]

While compile natively, the hosts cflags and ldflags are equal to ones
used from HOSTCFLAGS and HOSTLDFLAGS. When cross compiling it should
have own, used for target arch. While verification, for arm, arm64 and
x86_64 the following flags were used alsways:

-Wall
-O2
-fomit-frame-pointer
-Wmissing-prototypes
-Wstrict-prototypes

So, add them as they were verified and used before adding
Makefile.target, but anyway limit it only for cross compile options as
for host can be some configurations when another options can be used,
So, for host arch samples left all as is, it allows to avoid potential
option mistmatches for existent environments.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
 samples/bpf/Makefile | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 1579cc16a1c2..b5c87a8b8b51 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -178,8 +178,17 @@ CLANG_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR)
 TPROGS_CFLAGS += $(ARM_ARCH_SELECTOR)
 endif
 
+ifdef CROSS_COMPILE
+TPROGS_CFLAGS += -Wall
+TPROGS_CFLAGS += -O2
+TPROGS_CFLAGS += -fomit-frame-pointer
+TPROGS_CFLAGS += -Wmissing-prototypes
+TPROGS_CFLAGS += -Wstrict-prototypes
+else
 TPROGS_LDLIBS := $(KBUILD_HOSTLDLIBS)
 TPROGS_CFLAGS += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS)
+endif
+
 TPROGS_CFLAGS += -I$(objtree)/usr/include
 TPROGS_CFLAGS += -I$(srctree)/tools/lib/bpf/
 TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
-- 
2.17.1

[PATCH v3 bpf-next 10/14] samples: bpf: makefile: use target CC environment for HDR_PROBE

[Ivan Khoronzhuk]

No need in hacking HOSTCC to be cross-compiler any more, so drop
this trick and use target CC for HDR_PROBE.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
 samples/bpf/Makefile | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index b5c87a8b8b51..18ec22e7b444 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -214,15 +214,14 @@ BTF_PAHOLE ?= pahole
 
 # Detect that we're cross compiling and use the cross compiler
 ifdef CROSS_COMPILE
-HOSTCC = $(CROSS_COMPILE)gcc
 CLANG_ARCH_ARGS = --target=$(notdir $(CROSS_COMPILE:%-=%))
 endif
 
 # Don't evaluate probes and warnings if we need to run make recursively
 ifneq ($(src),)
 HDR_PROBE := $(shell printf "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \
-	$(HOSTCC) $(KBUILD_HOSTCFLAGS) -x c - -o /dev/null 2>/dev/null && \
-	echo okay)
+	$(CC) $(TPROGS_CFLAGS) $(TPROGS_LDFLAGS) -x c - \
+	-o /dev/null 2>/dev/null && echo okay)
 
 ifeq ($(HDR_PROBE),)
 $(warning WARNING: Detected possible issues with include path.)
-- 
2.17.1

[PATCH v3 bpf-next 11/14] libbpf: makefile: add C/CXX/LDFLAGS to libbpf.so and test_libpf targets

[Ivan Khoronzhuk]

In case of LDFLAGS and EXTRA_CC/CXX flags there is no way to pass them
correctly to build command, for instance when --sysroot is used or
external libraries are used, like -lelf, wich can be absent in
toolchain. This can be used for samples/bpf cross-compiling allowing
to get elf lib from sysroot.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
 tools/lib/bpf/Makefile | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index c6f94cffe06e..bccfa556ef4e 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@ -94,6 +94,10 @@ else
   CFLAGS := -g -Wall
 endif
 
+ifdef EXTRA_CXXFLAGS
+  CXXFLAGS := $(EXTRA_CXXFLAGS)
+endif
+
 ifeq ($(feature-libelf-mmap), 1)
   override CFLAGS += -DHAVE_LIBELF_MMAP_SUPPORT
 endif
@@ -176,8 +180,9 @@ $(BPF_IN): force elfdep bpfdep
 $(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION)
 
 $(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN)
-	$(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
-				    -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
+	$(QUIET_LINK)$(CC) $(LDFLAGS) \
+		--shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
+		-Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
 	@ln -sf $(@F) $(OUTPUT)libbpf.so
 	@ln -sf $(@F) $(OUTPUT)libbpf.so.$(LIBBPF_MAJOR_VERSION)
 
@@ -185,7 +190,7 @@ $(OUTPUT)libbpf.a: $(BPF_IN)
 	$(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
 
 $(OUTPUT)test_libbpf: test_libbpf.cpp $(OUTPUT)libbpf.a
-	$(QUIET_LINK)$(CXX) $(INCLUDES) $^ -lelf -o $@
+	$(QUIET_LINK)$(CXX) $(CXXFLAGS) $(LDFLAGS) $(INCLUDES) $^ -lelf -o $@
 
 $(OUTPUT)libbpf.pc:
 	$(QUIET_GEN)sed -e "s|@PREFIX@|$(prefix)|" \
-- 
2.17.1

[PATCH v3 bpf-next 12/14] samples: bpf: makefile: provide C/CXX/LD flags to libbpf

[Ivan Khoronzhuk]

In order to build libs using C/CXX/LD flags of target arch,
provide them to libbpf make.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
 samples/bpf/Makefile | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 18ec22e7b444..133123d4c7d7 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -182,8 +182,6 @@ ifdef CROSS_COMPILE
 TPROGS_CFLAGS += -Wall
 TPROGS_CFLAGS += -O2
 TPROGS_CFLAGS += -fomit-frame-pointer
-TPROGS_CFLAGS += -Wmissing-prototypes
-TPROGS_CFLAGS += -Wstrict-prototypes
 else
 TPROGS_LDLIBS := $(KBUILD_HOSTLDLIBS)
 TPROGS_CFLAGS += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS)
@@ -196,6 +194,14 @@ TPROGS_CFLAGS += -I$(srctree)/tools/lib/
 TPROGS_CFLAGS += -I$(srctree)/tools/include
 TPROGS_CFLAGS += -I$(srctree)/tools/perf
 
+EXTRA_CXXFLAGS := $(TPROGS_CFLAGS)
+
+# options not valid for C++
+ifdef CROSS_COMPILE
+$(TPROGS_CFLAGS) += -Wmissing-prototypes
+$(TPROGS_CFLAGS) += -Wstrict-prototypes
+endif
+
 TPROGCFLAGS_bpf_load.o += -Wno-unused-variable
 
 TPROGS_LDLIBS			+= $(LIBBPF) -lelf
@@ -257,7 +263,9 @@ clean:
 
 $(LIBBPF): FORCE
 # Fix up variables inherited from Kbuild that tools/ build system won't like
-	$(MAKE) -C $(dir $@) RM='rm -rf' LDFLAGS= srctree=$(BPF_SAMPLES_PATH)/../../ O=
+	$(MAKE) -C $(dir $@) RM='rm -rf' EXTRA_CFLAGS="$(TPROGS_CFLAGS)" \
+		EXTRA_CXXFLAGS="$(EXTRA_CXXFLAGS)" LDFLAGS=$(TPROGS_LDFLAGS) \
+		srctree=$(BPF_SAMPLES_PATH)/../../ O=
 
 $(obj)/syscall_nrs.h:	$(obj)/syscall_nrs.s FORCE
 	$(call filechk,offsets,__SYSCALL_NRS_H__)
-- 
2.17.1

[PATCH v3 bpf-next 13/14] samples: bpf: makefile: add sysroot support

[Ivan Khoronzhuk]

Basically it only enables that was added by previous couple fixes.
Sysroot contains correct libs installed and its headers ofc. Useful
when working with NFC or virtual machine.

Usage:

clean (on demand)
    make ARCH=arm -C samples/bpf clean
    make ARCH=arm -C tools clean
    make ARCH=arm clean

configure and install headers:

    make ARCH=arm defconfig
    make ARCH=arm headers_install

build samples/bpf:
    make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- samples/bpf/ \
    SYSROOT="path/to/sysroot"

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
 samples/bpf/Makefile | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 133123d4c7d7..57ddf055d6c3 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -194,6 +194,11 @@ TPROGS_CFLAGS += -I$(srctree)/tools/lib/
 TPROGS_CFLAGS += -I$(srctree)/tools/include
 TPROGS_CFLAGS += -I$(srctree)/tools/perf
 
+ifdef SYSROOT
+TPROGS_CFLAGS += --sysroot=${SYSROOT}
+TPROGS_LDFLAGS := -L${SYSROOT}/usr/lib
+endif
+
 EXTRA_CXXFLAGS := $(TPROGS_CFLAGS)
 
 # options not valid for C++
-- 
2.17.1

[PATCH v3 bpf-next 14/14] samples: bpf: README: add preparation steps and sysroot info

[Ivan Khoronzhuk]

Add couple preparation steps: clean and configuration. Also add newly
added sysroot support info to cross-compile section.
---
 samples/bpf/README.rst | 41 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 5 deletions(-)

diff --git a/samples/bpf/README.rst b/samples/bpf/README.rst
index 5f27e4faca50..d5845d73ab7d 100644
--- a/samples/bpf/README.rst
+++ b/samples/bpf/README.rst
@@ -14,6 +14,20 @@ Compiling requires having installed:
 Note that LLVM's tool 'llc' must support target 'bpf', list version
 and supported targets with command: ``llc --version``
 
+Clean and configuration
+-----------------------
+
+It can be needed to clean tools, samples or kernel before trying new arch or
+after some changes (on demand)::
+
+ make -C tools clean
+ make -C samples/bpf clean
+ make clean
+
+Configure kernel, defconfig for instance::
+
+ make defconfig
+
 Kernel headers
 --------------
 
@@ -68,9 +82,26 @@ It is also possible to point make to the newly compiled 'llc' or
 Cross compiling samples
 -----------------------
 In order to cross-compile, say for arm64 targets, export CROSS_COMPILE and ARCH
-environment variables before calling make. This will direct make to build
-samples for the cross target.
+environment variables before calling make. But do this before clean,
+cofiguration and header install steps described above. This will direct make to
+build samples for the cross target::
+
+ export ARCH=arm64
+ export CROSS_COMPILE="aarch64-linux-gnu-"
+
+Headers can be also installed on RFC of target board if need to keep them in
+sync (not necessarily and it creates a local "usr/include" directory also)::
+
+ make INSTALL_HDR_PATH=~/some_sysroot/usr headers_install
+
+Pointing LLC and CLANG is not necessarily if it's installed on HOST and have
+in its targets appropriate arm64 arch (usually it has several arches).
+Build samples::
+
+ make samples/bpf/
+
+Or build samples with SYSROOT if some header or library is absent in toolchain,
+say libelf, providing address to file system containing headers and libs,
+can be RFS of target board::
 
-export ARCH=arm64
-export CROSS_COMPILE="aarch64-linux-gnu-"
-make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
+ make samples/bpf/ SYSROOT=~/some_sysroot
-- 
2.17.1

Re: [PATCH v3 bpf-next 01/14] samples: bpf: makefile: fix HDR_PROBE "echo"

[Andrii Nakryiko]

On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> echo should be replaced with echo -e to handle '\n' correctly, but
> instead, replace it with printf as some systems can't handle echo -e.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
>  samples/bpf/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 1d9be26b4edd..f50ca852c2a8 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -201,7 +201,7 @@ endif
>
>  # Don't evaluate probes and warnings if we need to run make recursively
>  ifneq ($(src),)
> -HDR_PROBE := $(shell echo "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \
> +HDR_PROBE := $(shell printf "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \

printf change is fine, but I'm confused about \# at the beginning of
the string. Not sure what was the intent, but it seems like it should
work with just #include at the beginning.

>         $(HOSTCC) $(KBUILD_HOSTCFLAGS) -x c - -o /dev/null 2>/dev/null && \
>         echo okay)
>
> --
> 2.17.1
>

Re: [PATCH v3 bpf-next 02/14] samples: bpf: makefile: fix cookie_uid_helper_example obj build

[Andrii Nakryiko]

On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> Don't list userspace "cookie_uid_helper_example" object in list for
> bpf objects.
>
> 'always' target is used for listing bpf programs, but
> 'cookie_uid_helper_example.o' is a user space ELF file, and covered
> by rule `per_socket_stats_example`, so shouldn't be in 'always'.
> Let us remove `always += cookie_uid_helper_example.o`, which avoids
> breaking cross compilation due to mismatched includes.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---

Acked-by: Andrii Nakryiko <andriin@fb.com>

>  samples/bpf/Makefile | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index f50ca852c2a8..43dee90dffa4 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -145,7 +145,6 @@ always += sampleip_kern.o
>  always += lwt_len_hist_kern.o
>  always += xdp_tx_iptunnel_kern.o
>  always += test_map_in_map_kern.o
> -always += cookie_uid_helper_example.o
>  always += tcp_synrto_kern.o
>  always += tcp_rwnd_kern.o
>  always += tcp_bufs_kern.o
> --
> 2.17.1
>

Re: [PATCH v3 bpf-next 03/14] samples: bpf: makefile: use --target from cross-compile

[Andrii Nakryiko]

On Mon, Sep 16, 2019 at 4:01 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> For cross compiling the target triple can be inherited from
> cross-compile prefix as it's done in CLANG_FLAGS from kernel makefile.
> So copy-paste this decision from kernel Makefile.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---

Acked-by: Andrii Nakryiko <andriin@fb.com>

>  samples/bpf/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 43dee90dffa4..b59e77e2250e 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -195,7 +195,7 @@ BTF_PAHOLE ?= pahole
>  # Detect that we're cross compiling and use the cross compiler
>  ifdef CROSS_COMPILE
>  HOSTCC = $(CROSS_COMPILE)gcc
> -CLANG_ARCH_ARGS = -target $(ARCH)
> +CLANG_ARCH_ARGS = --target=$(notdir $(CROSS_COMPILE:%-=%))
>  endif
>
>  # Don't evaluate probes and warnings if we need to run make recursively
> --
> 2.17.1
>

Re: [PATCH v3 bpf-next 04/14] samples: bpf: use own EXTRA_CFLAGS for clang commands

[Andrii Nakryiko]

On Mon, Sep 16, 2019 at 4:01 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> It can overlap with CFLAGS used for libraries built with gcc if
> not now then in next patches. Correct it here for simplicity.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---

With GCC BPF front-end recently added, we should probably generalize
this to something like BPF_EXTRA_CFLAGS or something like that,
eventually. But for now:

Acked-by: Andrii Nakryiko <andriin@fb.com>

>  samples/bpf/Makefile | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index b59e77e2250e..8ecc5d0c2d5b 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -218,10 +218,10 @@ BTF_LLVM_PROBE := $(shell echo "int main() { return 0; }" | \
>                           /bin/rm -f ./llvm_btf_verify.o)
>
>  ifneq ($(BTF_LLVM_PROBE),)
> -       EXTRA_CFLAGS += -g
> +       CLANG_EXTRA_CFLAGS += -g
>  else
>  ifneq ($(and $(BTF_LLC_PROBE),$(BTF_PAHOLE_PROBE),$(BTF_OBJCOPY_PROBE)),)
> -       EXTRA_CFLAGS += -g
> +       CLANG_EXTRA_CFLAGS += -g
>         LLC_FLAGS += -mattr=dwarfris
>         DWARF2BTF = y
>  endif
> @@ -280,8 +280,8 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>  # useless for BPF samples.
>  $(obj)/%.o: $(src)/%.c
>         @echo "  CLANG-bpf " $@
> -       $(Q)$(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) -I$(obj) \
> -               -I$(srctree)/tools/testing/selftests/bpf/ \
> +       $(Q)$(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(CLANG_EXTRA_CFLAGS) \
> +               -I$(obj) -I$(srctree)/tools/testing/selftests/bpf/ \
>                 -D__KERNEL__ -D__BPF_TRACING__ -Wno-unused-value -Wno-pointer-sign \
>                 -D__TARGET_ARCH_$(SRCARCH) -Wno-compare-distinct-pointer-types \
>                 -Wno-gnu-variable-sized-type-not-at-end \
> --
> 2.17.1
>

Re: [PATCH v3 bpf-next 05/14] samples: bpf: makefile: use __LINUX_ARM_ARCH__ selector for arm

[Andrii Nakryiko]

On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> For arm, -D__LINUX_ARM_ARCH__=X is min version used as instruction
> set selector and is absolutely required while parsing some parts of
> headers. It's present in KBUILD_CFLAGS but not in autoconf.h, so let's
> retrieve it from and add to programs cflags. In another case errors
> like "SMP is not supported" for armv7 and bunch of other errors are
> issued resulting to incorrect final object.
> ---
>  samples/bpf/Makefile | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 8ecc5d0c2d5b..d3c8db3df560 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -185,6 +185,16 @@ HOSTLDLIBS_map_perf_test   += -lrt
>  HOSTLDLIBS_test_overhead       += -lrt
>  HOSTLDLIBS_xdpsock             += -pthread
>
> +ifeq ($(ARCH), arm)
> +# Strip all except -D__LINUX_ARM_ARCH__ option needed to handle linux
> +# headers when arm instruction set identification is requested.
> +ARM_ARCH_SELECTOR = $(shell echo "$(KBUILD_CFLAGS) " | \
> +                   sed 's/[[:blank:]]/\n/g' | sed '/^-D__LINUX_ARM_ARCH__/!d')

Does the following work exactly like that without shelling out (and
being arguably simpler)?

ARM_ARCH_SELECTOR = $(filter -D__LINUX_ARM_ARCH__%, $(KBUILD_CFLAGS))

> +
> +CLANG_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR)
> +KBUILD_HOSTCFLAGS := $(ARM_ARCH_SELECTOR)

Isn't this clearing out previous value of KBUILD_HOSTCFLAGS? Is that
intentional, or it was supposed to be += here?

> +endif
> +
>  # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline:
>  #  make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
>  LLC ?= llc
> --
> 2.17.1
>

Re: [PATCH v3 bpf-next 06/14] samples: bpf: makefile: drop unnecessarily inclusion for bpf_load

[Andrii Nakryiko]

On Mon, Sep 16, 2019 at 4:01 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> Drop inclusion for bpf_load -I$(objtree)/usr/include as it is
> included for all objects anyway, with above line:
> KBUILD_HOSTCFLAGS += -I$(objtree)/usr/include
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---

Acked-by: Andrii Nakryiko <andriin@fb.com>

>  samples/bpf/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index d3c8db3df560..9d923546e087 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -176,7 +176,7 @@ KBUILD_HOSTCFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
>  KBUILD_HOSTCFLAGS += -I$(srctree)/tools/lib/ -I$(srctree)/tools/include
>  KBUILD_HOSTCFLAGS += -I$(srctree)/tools/perf
>
> -HOSTCFLAGS_bpf_load.o += -I$(objtree)/usr/include -Wno-unused-variable
> +HOSTCFLAGS_bpf_load.o += -Wno-unused-variable
>
>  KBUILD_HOSTLDLIBS              += $(LIBBPF) -lelf
>  HOSTLDLIBS_tracex4             += -lrt
> --
> 2.17.1
>

Re: [PATCH v3 bpf-next 01/14] samples: bpf: makefile: fix HDR_PROBE "echo"

[Ivan Khoronzhuk]

On Mon, Sep 16, 2019 at 01:13:23PM -0700, Andrii Nakryiko wrote:
>On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk
><ivan.khoronzhuk@linaro.org> wrote:
>>
>> echo should be replaced with echo -e to handle '\n' correctly, but
>> instead, replace it with printf as some systems can't handle echo -e.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>>  samples/bpf/Makefile | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> index 1d9be26b4edd..f50ca852c2a8 100644
>> --- a/samples/bpf/Makefile
>> +++ b/samples/bpf/Makefile
>> @@ -201,7 +201,7 @@ endif
>>
>>  # Don't evaluate probes and warnings if we need to run make recursively
>>  ifneq ($(src),)
>> -HDR_PROBE := $(shell echo "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \
>> +HDR_PROBE := $(shell printf "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \
>
>printf change is fine, but I'm confused about \# at the beginning of
>the string. Not sure what was the intent, but it seems like it should
>work with just #include at the beginning.

At least no warns, but looks like should work.
Will try it in next v.

>
>>         $(HOSTCC) $(KBUILD_HOSTCFLAGS) -x c - -o /dev/null 2>/dev/null && \
>>         echo okay)
>>
>> --
>> 2.17.1
>>

-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH v3 bpf-next 01/14] samples: bpf: makefile: fix HDR_PROBE "echo"

[Andreas Schwab]

On Sep 16 2019, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:

> On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk
> <ivan.khoronzhuk@linaro.org> wrote:
>>
>> echo should be replaced with echo -e to handle '\n' correctly, but
>> instead, replace it with printf as some systems can't handle echo -e.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>>  samples/bpf/Makefile | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> index 1d9be26b4edd..f50ca852c2a8 100644
>> --- a/samples/bpf/Makefile
>> +++ b/samples/bpf/Makefile
>> @@ -201,7 +201,7 @@ endif
>>
>>  # Don't evaluate probes and warnings if we need to run make recursively
>>  ifneq ($(src),)
>> -HDR_PROBE := $(shell echo "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \
>> +HDR_PROBE := $(shell printf "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \
>
> printf change is fine, but I'm confused about \# at the beginning of
> the string.

From the NEWS of make 4.3:

* WARNING: Backward-incompatibility!
  Number signs (#) appearing inside a macro reference or function invocation
  no longer introduce comments and should not be escaped with backslashes:
  thus a call such as:
    foo := $(shell echo '#')
  is legal.  Previously the number sign needed to be escaped, for example:
    foo := $(shell echo '\#')
  Now this latter will resolve to "\#".  If you want to write makefiles
  portable to both versions, assign the number sign to a variable:
    H := \#
    foo := $(shell echo '$H')
  This was claimed to be fixed in 3.81, but wasn't, for some reason.
  To detect this change search for 'nocomment' in the .FEATURES variable.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH v3 bpf-next 04/14] samples: bpf: use own EXTRA_CFLAGS for clang commands

[Ivan Khoronzhuk]

On Mon, Sep 16, 2019 at 01:35:21PM -0700, Andrii Nakryiko wrote:
>On Mon, Sep 16, 2019 at 4:01 AM Ivan Khoronzhuk
><ivan.khoronzhuk@linaro.org> wrote:
>>
>> It can overlap with CFLAGS used for libraries built with gcc if
>> not now then in next patches. Correct it here for simplicity.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>
>With GCC BPF front-end recently added, we should probably generalize
>this to something like BPF_EXTRA_CFLAGS or something like that,
>eventually. But for now:
>
>Acked-by: Andrii Nakryiko <andriin@fb.com>

I can replace with BPF_EXTRA_CFLAGS in next v.

>
>>  samples/bpf/Makefile | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> index b59e77e2250e..8ecc5d0c2d5b 100644
>> --- a/samples/bpf/Makefile
>> +++ b/samples/bpf/Makefile
>> @@ -218,10 +218,10 @@ BTF_LLVM_PROBE := $(shell echo "int main() { return 0; }" | \
>>                           /bin/rm -f ./llvm_btf_verify.o)
>>
>>  ifneq ($(BTF_LLVM_PROBE),)
>> -       EXTRA_CFLAGS += -g
>> +       CLANG_EXTRA_CFLAGS += -g
>>  else
>>  ifneq ($(and $(BTF_LLC_PROBE),$(BTF_PAHOLE_PROBE),$(BTF_OBJCOPY_PROBE)),)
>> -       EXTRA_CFLAGS += -g
>> +       CLANG_EXTRA_CFLAGS += -g
>>         LLC_FLAGS += -mattr=dwarfris
>>         DWARF2BTF = y
>>  endif
>> @@ -280,8 +280,8 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>>  # useless for BPF samples.
>>  $(obj)/%.o: $(src)/%.c
>>         @echo "  CLANG-bpf " $@
>> -       $(Q)$(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) -I$(obj) \
>> -               -I$(srctree)/tools/testing/selftests/bpf/ \
>> +       $(Q)$(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(CLANG_EXTRA_CFLAGS) \
>> +               -I$(obj) -I$(srctree)/tools/testing/selftests/bpf/ \
>>                 -D__KERNEL__ -D__BPF_TRACING__ -Wno-unused-value -Wno-pointer-sign \
>>                 -D__TARGET_ARCH_$(SRCARCH) -Wno-compare-distinct-pointer-types \
>>                 -Wno-gnu-variable-sized-type-not-at-end \
>> --
>> 2.17.1
>>

-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH v3 bpf-next 01/14] samples: bpf: makefile: fix HDR_PROBE "echo"

[Andrii Nakryiko]

On Mon, Sep 16, 2019 at 2:35 PM Andreas Schwab <schwab@linux-m68k.org> wrote:
>
> On Sep 16 2019, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> > On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk
> > <ivan.khoronzhuk@linaro.org> wrote:
> >>
> >> echo should be replaced with echo -e to handle '\n' correctly, but
> >> instead, replace it with printf as some systems can't handle echo -e.
> >>
> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> >> ---
> >>  samples/bpf/Makefile | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> >> index 1d9be26b4edd..f50ca852c2a8 100644
> >> --- a/samples/bpf/Makefile
> >> +++ b/samples/bpf/Makefile
> >> @@ -201,7 +201,7 @@ endif
> >>
> >>  # Don't evaluate probes and warnings if we need to run make recursively
> >>  ifneq ($(src),)
> >> -HDR_PROBE := $(shell echo "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \
> >> +HDR_PROBE := $(shell printf "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \
> >
> > printf change is fine, but I'm confused about \# at the beginning of
> > the string.
>
> From the NEWS of make 4.3:
>
> * WARNING: Backward-incompatibility!
>   Number signs (#) appearing inside a macro reference or function invocation
>   no longer introduce comments and should not be escaped with backslashes:
>   thus a call such as:
>     foo := $(shell echo '#')
>   is legal.  Previously the number sign needed to be escaped, for example:
>     foo := $(shell echo '\#')
>   Now this latter will resolve to "\#".  If you want to write makefiles
>   portable to both versions, assign the number sign to a variable:
>     H := \#
>     foo := $(shell echo '$H')
>   This was claimed to be fixed in 3.81, but wasn't, for some reason.
>   To detect this change search for 'nocomment' in the .FEATURES variable.
>
> Andreas.

Oh, subtle... Thanks for explaining!

>
> --
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
> "And now for something completely different."

Re: [PATCH v3 bpf-next 05/14] samples: bpf: makefile: use __LINUX_ARM_ARCH__ selector for arm

[Ivan Khoronzhuk]

On Mon, Sep 16, 2019 at 01:44:23PM -0700, Andrii Nakryiko wrote:
>On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk
><ivan.khoronzhuk@linaro.org> wrote:
>>
>> For arm, -D__LINUX_ARM_ARCH__=X is min version used as instruction
>> set selector and is absolutely required while parsing some parts of
>> headers. It's present in KBUILD_CFLAGS but not in autoconf.h, so let's
>> retrieve it from and add to programs cflags. In another case errors
>> like "SMP is not supported" for armv7 and bunch of other errors are
>> issued resulting to incorrect final object.
>> ---
>>  samples/bpf/Makefile | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> index 8ecc5d0c2d5b..d3c8db3df560 100644
>> --- a/samples/bpf/Makefile
>> +++ b/samples/bpf/Makefile
>> @@ -185,6 +185,16 @@ HOSTLDLIBS_map_perf_test   += -lrt
>>  HOSTLDLIBS_test_overhead       += -lrt
>>  HOSTLDLIBS_xdpsock             += -pthread
>>
>> +ifeq ($(ARCH), arm)
>> +# Strip all except -D__LINUX_ARM_ARCH__ option needed to handle linux
>> +# headers when arm instruction set identification is requested.
>> +ARM_ARCH_SELECTOR = $(shell echo "$(KBUILD_CFLAGS) " | \
>> +                   sed 's/[[:blank:]]/\n/g' | sed '/^-D__LINUX_ARM_ARCH__/!d')
>
>Does the following work exactly like that without shelling out (and
>being arguably simpler)?
>
>ARM_ARCH_SELECTOR = $(filter -D__LINUX_ARM_ARCH__%, $(KBUILD_CFLAGS))
>
>> +
>> +CLANG_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR)
>> +KBUILD_HOSTCFLAGS := $(ARM_ARCH_SELECTOR)
>
>Isn't this clearing out previous value of KBUILD_HOSTCFLAGS? Is that
>intentional, or it was supposed to be += here?
>
>> +endif
>> +
>>  # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline:
>>  #  make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
>>  LLC ?= llc
>> --
>> 2.17.1
>>

Just left from previous version filtering all -D options.
Will update in next v., SELECTOR also.

-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH v3 bpf-next 09/14] samples: bpf: makefile: use own flags but not host when cross compile

[Sergei Shtylyov]

Hello!

On 16.09.2019 13:54, Ivan Khoronzhuk wrote:

> While compile natively, the hosts cflags and ldflags are equal to ones

   Compiling. Host's.

> used from HOSTCFLAGS and HOSTLDFLAGS. When cross compiling it should
> have own, used for target arch. While verification, for arm, arm64 and
> x86_64 the following flags were used alsways:
> 
> -Wall
> -O2
> -fomit-frame-pointer
> -Wmissing-prototypes
> -Wstrict-prototypes
> 
> So, add them as they were verified and used before adding
> Makefile.target, but anyway limit it only for cross compile options as
> for host can be some configurations when another options can be used,
> So, for host arch samples left all as is, it allows to avoid potential
> option mistmatches for existent environments.

    Mismatches.

> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
[...]

MBR, Sergei

Re: [PATCH v3 bpf-next 07/14] samples: bpf: add makefile.target for separate CC target build

[Andrii Nakryiko]

On Mon, Sep 16, 2019 at 3:58 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> The makefile.target is added only and will be used in

typo: Makefile

> sample/bpf/Makefile later in order to switch cross-compiling on CC

on -> to

> from HOSTCC environment.
>
> The HOSTCC is supposed to build binaries and tools running on the host
> afterwards, in order to simplify build or so, like "fixdep" or else.
> In case of cross compiling "fixdep" is executed on host when the rest
> samples should run on target arch. In order to build binaries for
> target arch with CC and tools running on host with HOSTCC, lets add
> Makefile.target for simplicity, having definition and routines similar
> to ones, used in script/Makefile.host. This allows later add
> cross-compilation to samples/bpf with minimum changes.
>
> The tprog stands for target programs built with CC.

Why tprog? Could we just use prog: hostprog vs prog.

>
> Makefile.target contains only stuff needed for samples/bpf, potentially
> can be reused later and now needed only for unblocking tricky
> samples/bpf cross compilation.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
>  samples/bpf/Makefile.target | 75 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 75 insertions(+)
>  create mode 100644 samples/bpf/Makefile.target
>
> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
> new file mode 100644
> index 000000000000..fb6de63f7d2f
> --- /dev/null
> +++ b/samples/bpf/Makefile.target
> @@ -0,0 +1,75 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# ==========================================================================
> +# Building binaries on the host system
> +# Binaries are not used during the compilation of the kernel, and intendent

typo: intended

> +# to be build for target board, target board can be host ofc. Added to build

What's ofc, is it "of course"?

> +# binaries to run not on host system.
> +#
> +# Sample syntax (see Documentation/kbuild/makefiles.rst for reference)
> +# tprogs-y := xsk_example
> +# Will compile xdpsock_example.c and create an executable named xsk_example

You mix references to xsk_example and xdpsock_example, which is very
confusing. I'm guessing you meant to use xdpsock_example consistently.

> +#
> +# tprogs-y    := xdpsock
> +# xdpsock-objs := xdpsock_1.o xdpsock_2.o
> +# Will compile xdpsock_1.c and xdpsock_2.c, and then link the executable
> +# xdpsock, based on xdpsock_1.o and xdpsock_2.o
> +#
> +# Inherited from scripts/Makefile.host

"Inspired by" or "Derived from" would be probably more appropriate term :)

> +#
> +__tprogs := $(sort $(tprogs-y))
> +
> +# C code
> +# Executables compiled from a single .c file
> +tprog-csingle  := $(foreach m,$(__tprogs), \
> +                       $(if $($(m)-objs),,$(m)))
> +
> +# C executables linked based on several .o files
> +tprog-cmulti   := $(foreach m,$(__tprogs),\
> +                       $(if $($(m)-objs),$(m)))
> +
> +# Object (.o) files compiled from .c files
> +tprog-cobjs    := $(sort $(foreach m,$(__tprogs),$($(m)-objs)))
> +
> +tprog-csingle  := $(addprefix $(obj)/,$(tprog-csingle))
> +tprog-cmulti   := $(addprefix $(obj)/,$(tprog-cmulti))
> +tprog-cobjs    := $(addprefix $(obj)/,$(tprog-cobjs))
> +
> +#####
> +# Handle options to gcc. Support building with separate output directory
> +
> +_tprogc_flags   = $(TPROGS_CFLAGS) \
> +                 $(TPROGCFLAGS_$(basetarget).o)
> +
> +# $(objtree)/$(obj) for including generated headers from checkin source files
> +ifeq ($(KBUILD_EXTMOD),)
> +ifdef building_out_of_srctree
> +_tprogc_flags   += -I $(objtree)/$(obj)
> +endif
> +endif
> +
> +tprogc_flags    = -Wp,-MD,$(depfile) $(_tprogc_flags)
> +
> +# Create executable from a single .c file
> +# tprog-csingle -> Executable
> +quiet_cmd_tprog-csingle        = CC  $@
> +      cmd_tprog-csingle        = $(CC) $(tprogc_flags) $(TPROGS_LDFLAGS) -o $@ $< \
> +               $(TPROGS_LDLIBS) $(TPROGLDLIBS_$(@F))
> +$(tprog-csingle): $(obj)/%: $(src)/%.c FORCE
> +       $(call if_changed_dep,tprog-csingle)
> +
> +# Link an executable based on list of .o files, all plain c
> +# tprog-cmulti -> executable
> +quiet_cmd_tprog-cmulti = LD  $@
> +      cmd_tprog-cmulti = $(CC) $(tprogc_flags) $(TPROGS_LDFLAGS) -o $@ \
> +                         $(addprefix $(obj)/,$($(@F)-objs)) \
> +                         $(TPROGS_LDLIBS) $(TPROGLDLIBS_$(@F))
> +$(tprog-cmulti): $(tprog-cobjs) FORCE
> +       $(call if_changed,tprog-cmulti)
> +$(call multi_depend, $(tprog-cmulti), , -objs)
> +
> +# Create .o file from a single .c file
> +# tprog-cobjs -> .o
> +quiet_cmd_tprog-cobjs  = CC  $@
> +      cmd_tprog-cobjs  = $(CC) $(tprogc_flags) -c -o $@ $<
> +$(tprog-cobjs): $(obj)/%.o: $(src)/%.c FORCE
> +       $(call if_changed_dep,tprog-cobjs)
> --
> 2.17.1
>

tprogs is quite confusing, but overall looks good to me.

Re: [PATCH v3 bpf-next 08/14] samples: bpf: makefile: base target programs rules on Makefile.target

[Andrii Nakryiko]

On Mon, Sep 16, 2019 at 3:58 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>

Please don't prepend "samples: bpf: makefile:" to patches,
"samples/bpf: " is a typical we've used for BPF samples changes.


> The main reason for that - HOSTCC and CC have different aims.
> HOSTCC is used to build programs running on host, that can
> cross-comple target programs with CC. It was tested for arm and arm64
> cross compilation, based on linaro toolchain, but should work for
> others.
>
> So, in order to split cross compilation (CC) with host build (HOSTCC),
> lets base samples on Makefile.target. It allows to cross-compile
> samples/bpf programs with CC while auxialry tools running on host
> built with HOSTCC.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
>  samples/bpf/Makefile | 135 ++++++++++++++++++++++---------------------
>  1 file changed, 69 insertions(+), 66 deletions(-)
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 9d923546e087..1579cc16a1c2 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -4,55 +4,53 @@ BPF_SAMPLES_PATH ?= $(abspath $(srctree)/$(src))
>  TOOLS_PATH := $(BPF_SAMPLES_PATH)/../../tools
>
>  # List of programs to build
> -hostprogs-y := test_lru_dist
> -hostprogs-y += sock_example
> -hostprogs-y += fds_example
> -hostprogs-y += sockex1
> -hostprogs-y += sockex2
> -hostprogs-y += sockex3
> -hostprogs-y += tracex1
> -hostprogs-y += tracex2
> -hostprogs-y += tracex3
> -hostprogs-y += tracex4
> -hostprogs-y += tracex5
> -hostprogs-y += tracex6
> -hostprogs-y += tracex7
> -hostprogs-y += test_probe_write_user
> -hostprogs-y += trace_output
> -hostprogs-y += lathist
> -hostprogs-y += offwaketime
> -hostprogs-y += spintest
> -hostprogs-y += map_perf_test
> -hostprogs-y += test_overhead
> -hostprogs-y += test_cgrp2_array_pin
> -hostprogs-y += test_cgrp2_attach
> -hostprogs-y += test_cgrp2_sock
> -hostprogs-y += test_cgrp2_sock2
> -hostprogs-y += xdp1
> -hostprogs-y += xdp2
> -hostprogs-y += xdp_router_ipv4
> -hostprogs-y += test_current_task_under_cgroup
> -hostprogs-y += trace_event
> -hostprogs-y += sampleip
> -hostprogs-y += tc_l2_redirect
> -hostprogs-y += lwt_len_hist
> -hostprogs-y += xdp_tx_iptunnel
> -hostprogs-y += test_map_in_map
> -hostprogs-y += per_socket_stats_example
> -hostprogs-y += xdp_redirect
> -hostprogs-y += xdp_redirect_map
> -hostprogs-y += xdp_redirect_cpu
> -hostprogs-y += xdp_monitor
> -hostprogs-y += xdp_rxq_info
> -hostprogs-y += syscall_tp
> -hostprogs-y += cpustat
> -hostprogs-y += xdp_adjust_tail
> -hostprogs-y += xdpsock
> -hostprogs-y += xdp_fwd
> -hostprogs-y += task_fd_query
> -hostprogs-y += xdp_sample_pkts
> -hostprogs-y += ibumad
> -hostprogs-y += hbm
> +tprogs-y := test_lru_dist
> +tprogs-y += sock_example
> +tprogs-y += fds_example
> +tprogs-y += sockex1
> +tprogs-y += sockex2
> +tprogs-y += sockex3
> +tprogs-y += tracex1
> +tprogs-y += tracex2
> +tprogs-y += tracex3
> +tprogs-y += tracex4
> +tprogs-y += tracex5
> +tprogs-y += tracex6
> +tprogs-y += tracex7
> +tprogs-y += test_probe_write_user
> +tprogs-y += trace_output
> +tprogs-y += lathist
> +tprogs-y += offwaketime
> +tprogs-y += spintest
> +tprogs-y += map_perf_test
> +tprogs-y += test_overhead
> +tprogs-y += test_cgrp2_array_pin
> +tprogs-y += test_cgrp2_attach
> +tprogs-y += test_cgrp2_sock
> +tprogs-y += test_cgrp2_sock2
> +tprogs-y += xdp1
> +tprogs-y += xdp2
> +tprogs-y += xdp_router_ipv4
> +tprogs-y += test_current_task_under_cgroup
> +tprogs-y += trace_event
> +tprogs-y += sampleip
> +tprogs-y += tc_l2_redirect
> +tprogs-y += lwt_len_hist
> +tprogs-y += xdp_tx_iptunnel
> +tprogs-y += test_map_in_map
> +tprogs-y += xdp_redirect_map
> +tprogs-y += xdp_redirect_cpu
> +tprogs-y += xdp_monitor
> +tprogs-y += xdp_rxq_info
> +tprogs-y += syscall_tp
> +tprogs-y += cpustat
> +tprogs-y += xdp_adjust_tail
> +tprogs-y += xdpsock
> +tprogs-y += xdp_fwd
> +tprogs-y += task_fd_query
> +tprogs-y += xdp_sample_pkts
> +tprogs-y += ibumad
> +tprogs-y += hbm
>
>  # Libbpf dependencies
>  LIBBPF = $(TOOLS_PATH)/lib/bpf/libbpf.a
> @@ -111,7 +109,7 @@ ibumad-objs := bpf_load.o ibumad_user.o $(TRACE_HELPERS)
>  hbm-objs := bpf_load.o hbm.o $(CGROUP_HELPERS)
>
>  # Tell kbuild to always build the programs
> -always := $(hostprogs-y)
> +always := $(tprogs-y)
>  always += sockex1_kern.o
>  always += sockex2_kern.o
>  always += sockex3_kern.o
> @@ -170,21 +168,6 @@ always += ibumad_kern.o
>  always += hbm_out_kern.o
>  always += hbm_edt_kern.o
>
> -KBUILD_HOSTCFLAGS += -I$(objtree)/usr/include
> -KBUILD_HOSTCFLAGS += -I$(srctree)/tools/lib/bpf/
> -KBUILD_HOSTCFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
> -KBUILD_HOSTCFLAGS += -I$(srctree)/tools/lib/ -I$(srctree)/tools/include
> -KBUILD_HOSTCFLAGS += -I$(srctree)/tools/perf
> -
> -HOSTCFLAGS_bpf_load.o += -Wno-unused-variable
> -
> -KBUILD_HOSTLDLIBS              += $(LIBBPF) -lelf
> -HOSTLDLIBS_tracex4             += -lrt
> -HOSTLDLIBS_trace_output        += -lrt
> -HOSTLDLIBS_map_perf_test       += -lrt
> -HOSTLDLIBS_test_overhead       += -lrt
> -HOSTLDLIBS_xdpsock             += -pthread
> -
>  ifeq ($(ARCH), arm)
>  # Strip all except -D__LINUX_ARM_ARCH__ option needed to handle linux
>  # headers when arm instruction set identification is requested.
> @@ -192,9 +175,27 @@ ARM_ARCH_SELECTOR = $(shell echo "$(KBUILD_CFLAGS) " | \
>                     sed 's/[[:blank:]]/\n/g' | sed '/^-D__LINUX_ARM_ARCH__/!d')
>
>  CLANG_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR)
> -KBUILD_HOSTCFLAGS := $(ARM_ARCH_SELECTOR)
> +TPROGS_CFLAGS += $(ARM_ARCH_SELECTOR)
>  endif
>
> +TPROGS_LDLIBS := $(KBUILD_HOSTLDLIBS)

Please group TPROGS_LDLIBS definition together with the one below,
there doesn't seem to be a reason to split them this way.

But also, it's kind of weird to use host libraries as cross-compiled
libraries as well. Is that intentional?

> +TPROGS_CFLAGS += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS)

Same here, is it right to use HOSTCFLAGS and HOST_EXTRACFLAGS as a
base for cross-compiled cflags?

> +TPROGS_CFLAGS += -I$(objtree)/usr/include
> +TPROGS_CFLAGS += -I$(srctree)/tools/lib/bpf/
> +TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
> +TPROGS_CFLAGS += -I$(srctree)/tools/lib/
> +TPROGS_CFLAGS += -I$(srctree)/tools/include
> +TPROGS_CFLAGS += -I$(srctree)/tools/perf
> +
> +TPROGCFLAGS_bpf_load.o += -Wno-unused-variable
> +
> +TPROGS_LDLIBS                  += $(LIBBPF) -lelf
> +TPROGLDLIBS_tracex4            += -lrt
> +TPROGLDLIBS_trace_output       += -lrt
> +TPROGLDLIBS_map_perf_test      += -lrt
> +TPROGLDLIBS_test_overhead      += -lrt
> +TPROGLDLIBS_xdpsock            += -pthread
> +
>  # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline:
>  #  make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
>  LLC ?= llc
> @@ -285,6 +286,8 @@ $(obj)/hbm_out_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>  $(obj)/hbm.o: $(src)/hbm.h
>  $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>
> +-include $(BPF_SAMPLES_PATH)/Makefile.target
> +
>  # asm/sysreg.h - inline assembly used by it is incompatible with llvm.
>  # But, there is no easy way to fix it, so just exclude it since it is
>  # useless for BPF samples.
> --
> 2.17.1
>

Re: [PATCH v3 bpf-next 09/14] samples: bpf: makefile: use own flags but not host when cross compile

[Andrii Nakryiko]

On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> While compile natively, the hosts cflags and ldflags are equal to ones
> used from HOSTCFLAGS and HOSTLDFLAGS. When cross compiling it should
> have own, used for target arch. While verification, for arm, arm64 and
> x86_64 the following flags were used alsways:
>
> -Wall
> -O2
> -fomit-frame-pointer
> -Wmissing-prototypes
> -Wstrict-prototypes
>
> So, add them as they were verified and used before adding
> Makefile.target, but anyway limit it only for cross compile options as
> for host can be some configurations when another options can be used,
> So, for host arch samples left all as is, it allows to avoid potential
> option mistmatches for existent environments.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
>  samples/bpf/Makefile | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 1579cc16a1c2..b5c87a8b8b51 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -178,8 +178,17 @@ CLANG_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR)
>  TPROGS_CFLAGS += $(ARM_ARCH_SELECTOR)
>  endif
>
> +ifdef CROSS_COMPILE
> +TPROGS_CFLAGS += -Wall
> +TPROGS_CFLAGS += -O2

Specifying one arg per line seems like overkill, put them in one line?

> +TPROGS_CFLAGS += -fomit-frame-pointer

Why this one?

> +TPROGS_CFLAGS += -Wmissing-prototypes
> +TPROGS_CFLAGS += -Wstrict-prototypes

Are these in some way special that we want them in cross-compile mode only?

All of those flags seem useful regardless of cross-compilation or not,
shouldn't they be common? I'm a bit lost about the intent here...

> +else
>  TPROGS_LDLIBS := $(KBUILD_HOSTLDLIBS)
>  TPROGS_CFLAGS += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS)
> +endif
> +
>  TPROGS_CFLAGS += -I$(objtree)/usr/include
>  TPROGS_CFLAGS += -I$(srctree)/tools/lib/bpf/
>  TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
> --
> 2.17.1
>

Re: [PATCH v3 bpf-next 10/14] samples: bpf: makefile: use target CC environment for HDR_PROBE

[Andrii Nakryiko]

On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> No need in hacking HOSTCC to be cross-compiler any more, so drop
> this trick and use target CC for HDR_PROBE.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---

Acked-by: Andrii Nakryiko <andriin@fb.com>


[...]

Re: [PATCH v3 bpf-next 11/14] libbpf: makefile: add C/CXX/LDFLAGS to libbpf.so and test_libpf targets

[Andrii Nakryiko]

On Mon, Sep 16, 2019 at 4:00 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> In case of LDFLAGS and EXTRA_CC/CXX flags there is no way to pass them
> correctly to build command, for instance when --sysroot is used or
> external libraries are used, like -lelf, wich can be absent in
> toolchain. This can be used for samples/bpf cross-compiling allowing
> to get elf lib from sysroot.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
>  tools/lib/bpf/Makefile | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> index c6f94cffe06e..bccfa556ef4e 100644
> --- a/tools/lib/bpf/Makefile
> +++ b/tools/lib/bpf/Makefile
> @@ -94,6 +94,10 @@ else
>    CFLAGS := -g -Wall
>  endif
>
> +ifdef EXTRA_CXXFLAGS
> +  CXXFLAGS := $(EXTRA_CXXFLAGS)
> +endif
> +
>  ifeq ($(feature-libelf-mmap), 1)
>    override CFLAGS += -DHAVE_LIBELF_MMAP_SUPPORT
>  endif
> @@ -176,8 +180,9 @@ $(BPF_IN): force elfdep bpfdep
>  $(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION)
>
>  $(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN)
> -       $(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
> -                                   -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
> +       $(QUIET_LINK)$(CC) $(LDFLAGS) \
> +               --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
> +               -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
>         @ln -sf $(@F) $(OUTPUT)libbpf.so
>         @ln -sf $(@F) $(OUTPUT)libbpf.so.$(LIBBPF_MAJOR_VERSION)
>
> @@ -185,7 +190,7 @@ $(OUTPUT)libbpf.a: $(BPF_IN)
>         $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
>
>  $(OUTPUT)test_libbpf: test_libbpf.cpp $(OUTPUT)libbpf.a
> -       $(QUIET_LINK)$(CXX) $(INCLUDES) $^ -lelf -o $@
> +       $(QUIET_LINK)$(CXX) $(CXXFLAGS) $(LDFLAGS) $(INCLUDES) $^ -lelf -o $@

Instead of doing ifdef EXTRA_CXXFLAGS bit above, you can just include
both $(CXXFLAGS) and $(EXTRA_CXXFLAGS), which will do the right thing
(and is actually recommended my make documentation way to do this).

But actually, there is no need to use C++ compiler here,
test_libbpf.cpp can just be plain C. Do you mind renaming it to .c and
using C compiler instead?

>
>  $(OUTPUT)libbpf.pc:
>         $(QUIET_GEN)sed -e "s|@PREFIX@|$(prefix)|" \
> --
> 2.17.1
>

Re: [PATCH v3 bpf-next 12/14] samples: bpf: makefile: provide C/CXX/LD flags to libbpf

[Andrii Nakryiko]

On Mon, Sep 16, 2019 at 3:58 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> In order to build libs using C/CXX/LD flags of target arch,
> provide them to libbpf make.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
>  samples/bpf/Makefile | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 18ec22e7b444..133123d4c7d7 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -182,8 +182,6 @@ ifdef CROSS_COMPILE
>  TPROGS_CFLAGS += -Wall
>  TPROGS_CFLAGS += -O2
>  TPROGS_CFLAGS += -fomit-frame-pointer
> -TPROGS_CFLAGS += -Wmissing-prototypes
> -TPROGS_CFLAGS += -Wstrict-prototypes
>  else
>  TPROGS_LDLIBS := $(KBUILD_HOSTLDLIBS)
>  TPROGS_CFLAGS += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS)
> @@ -196,6 +194,14 @@ TPROGS_CFLAGS += -I$(srctree)/tools/lib/
>  TPROGS_CFLAGS += -I$(srctree)/tools/include
>  TPROGS_CFLAGS += -I$(srctree)/tools/perf
>
> +EXTRA_CXXFLAGS := $(TPROGS_CFLAGS)
> +
> +# options not valid for C++
> +ifdef CROSS_COMPILE
> +$(TPROGS_CFLAGS) += -Wmissing-prototypes
> +$(TPROGS_CFLAGS) += -Wstrict-prototypes
> +endif
> +

ugh, let's really get rid of dependency on C++ compiler, as suggested
for previous patch.


>  TPROGCFLAGS_bpf_load.o += -Wno-unused-variable
>
>  TPROGS_LDLIBS                  += $(LIBBPF) -lelf
> @@ -257,7 +263,9 @@ clean:
>
>  $(LIBBPF): FORCE
>  # Fix up variables inherited from Kbuild that tools/ build system won't like
> -       $(MAKE) -C $(dir $@) RM='rm -rf' LDFLAGS= srctree=$(BPF_SAMPLES_PATH)/../../ O=
> +       $(MAKE) -C $(dir $@) RM='rm -rf' EXTRA_CFLAGS="$(TPROGS_CFLAGS)" \
> +               EXTRA_CXXFLAGS="$(EXTRA_CXXFLAGS)" LDFLAGS=$(TPROGS_LDFLAGS) \
> +               srctree=$(BPF_SAMPLES_PATH)/../../ O=
>
>  $(obj)/syscall_nrs.h:  $(obj)/syscall_nrs.s FORCE
>         $(call filechk,offsets,__SYSCALL_NRS_H__)
> --
> 2.17.1
>

Re: [PATCH v3 bpf-next 13/14] samples: bpf: makefile: add sysroot support

[Andrii Nakryiko]

On Mon, Sep 16, 2019 at 4:00 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> Basically it only enables that was added by previous couple fixes.
> Sysroot contains correct libs installed and its headers ofc. Useful

Please, let's not use unnecessary abbreviations/slang. "Of course" is
not too long and is a proper English, let's stick to it.

> when working with NFC or virtual machine.
>
> Usage:
>
> clean (on demand)
>     make ARCH=arm -C samples/bpf clean
>     make ARCH=arm -C tools clean
>     make ARCH=arm clean
>
> configure and install headers:
>
>     make ARCH=arm defconfig
>     make ARCH=arm headers_install
>
> build samples/bpf:
>     make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- samples/bpf/ \
>     SYSROOT="path/to/sysroot"
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
>  samples/bpf/Makefile | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 133123d4c7d7..57ddf055d6c3 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -194,6 +194,11 @@ TPROGS_CFLAGS += -I$(srctree)/tools/lib/
>  TPROGS_CFLAGS += -I$(srctree)/tools/include
>  TPROGS_CFLAGS += -I$(srctree)/tools/perf
>
> +ifdef SYSROOT
> +TPROGS_CFLAGS += --sysroot=${SYSROOT}
> +TPROGS_LDFLAGS := -L${SYSROOT}/usr/lib

Please stay consistent: $() instead of ${}?

> +endif
> +
>  EXTRA_CXXFLAGS := $(TPROGS_CFLAGS)
>
>  # options not valid for C++
> --
> 2.17.1
>

Re: [PATCH v3 bpf-next 00/14] samples: bpf: improve/fix cross-compilation

[Andrii Nakryiko]

On Mon, Sep 16, 2019 at 4:02 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>

Thanks for these changes, they look good overall. It would be great if
someone else could test and validate that cross-compilation works not
just in your environment and generated binaries successfully run on
target machines, though...

[...]


>
> Ivan Khoronzhuk (14):
>   samples: bpf: makefile: fix HDR_PROBE "echo"
>   samples: bpf: makefile: fix cookie_uid_helper_example obj build
>   samples: bpf: makefile: use --target from cross-compile
>   samples: bpf: use own EXTRA_CFLAGS for clang commands
>   samples: bpf: makefile: use __LINUX_ARM_ARCH__ selector for arm
>   samples: bpf: makefile: drop unnecessarily inclusion for bpf_load
>   samples: bpf: add makefile.target for separate CC target build
>   samples: bpf: makefile: base target programs rules on Makefile.target
>   samples: bpf: makefile: use own flags but not host when cross compile
>   samples: bpf: makefile: use target CC environment for HDR_PROBE
>   libbpf: makefile: add C/CXX/LDFLAGS to libbpf.so and test_libpf
>     targets
>   samples: bpf: makefile: provide C/CXX/LD flags to libbpf
>   samples: bpf: makefile: add sysroot support
>   samples: bpf: README: add preparation steps and sysroot info
>

Prefixes like "samples: bpf: makefile: " are very verbose without
adding much value. We've been converging to essentially this set of
prefixes:

- "libbpf:" for libbpf changes
- "bpftool:" for bpftool changes
- "selftests/bpf:" for bpf selftests
- "samples/bpf:" for bpf samples

There is no need to prefix with "makefile: " either. Please update
your patch subjects in the next version. Thanks!

>  samples/bpf/Makefile        | 179 +++++++++++++++++++++---------------
>  samples/bpf/Makefile.target |  75 +++++++++++++++
>  samples/bpf/README.rst      |  41 ++++++++-
>  tools/lib/bpf/Makefile      |  11 ++-
>  4 files changed, 225 insertions(+), 81 deletions(-)
>  create mode 100644 samples/bpf/Makefile.target
>
> --
> 2.17.1
>

Re: [PATCH v3 bpf-next 07/14] samples: bpf: add makefile.target for separate CC target build

[Ivan Khoronzhuk]

On Tue, Sep 17, 2019 at 04:19:40PM -0700, Andrii Nakryiko wrote:
>On Mon, Sep 16, 2019 at 3:58 AM Ivan Khoronzhuk
><ivan.khoronzhuk@linaro.org> wrote:
>>
>> The makefile.target is added only and will be used in
>
>typo: Makefile
>
>> sample/bpf/Makefile later in order to switch cross-compiling on CC
>
>on -> to
>
>> from HOSTCC environment.
>>
>> The HOSTCC is supposed to build binaries and tools running on the host
>> afterwards, in order to simplify build or so, like "fixdep" or else.
>> In case of cross compiling "fixdep" is executed on host when the rest
>> samples should run on target arch. In order to build binaries for
>> target arch with CC and tools running on host with HOSTCC, lets add
>> Makefile.target for simplicity, having definition and routines similar
>> to ones, used in script/Makefile.host. This allows later add
>> cross-compilation to samples/bpf with minimum changes.
>>
>> The tprog stands for target programs built with CC.
>
>Why tprog? Could we just use prog: hostprog vs prog.
Prev. version was with prog, but Yonghong Song found it ambiguous.
As prog can be bpf also. So, decision was made to follow logic:
* target prog - non bpf progs
* bpf prog = bpf prog, that can be later smth similar, providing build options
  for each bpf object separately.

Details here:
https://lkml.org/lkml/2019/9/13/1037

>
>>
>> Makefile.target contains only stuff needed for samples/bpf, potentially
>> can be reused later and now needed only for unblocking tricky
>> samples/bpf cross compilation.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>>  samples/bpf/Makefile.target | 75 +++++++++++++++++++++++++++++++++++++
>>  1 file changed, 75 insertions(+)
>>  create mode 100644 samples/bpf/Makefile.target
>>
>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
>> new file mode 100644
>> index 000000000000..fb6de63f7d2f
>> --- /dev/null
>> +++ b/samples/bpf/Makefile.target
>> @@ -0,0 +1,75 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +# ==========================================================================
>> +# Building binaries on the host system
>> +# Binaries are not used during the compilation of the kernel, and intendent
>
>typo: intended
>
>> +# to be build for target board, target board can be host ofc. Added to build
>
>What's ofc, is it "of course"?
yes, ofc )

>
>> +# binaries to run not on host system.
>> +#
>> +# Sample syntax (see Documentation/kbuild/makefiles.rst for reference)
>> +# tprogs-y := xsk_example
>> +# Will compile xdpsock_example.c and create an executable named xsk_example
>
>You mix references to xsk_example and xdpsock_example, which is very
>confusing. I'm guessing you meant to use xdpsock_example consistently.
Oh, yes. Thanks.

>
>> +#
>> +# tprogs-y    := xdpsock
>> +# xdpsock-objs := xdpsock_1.o xdpsock_2.o
>> +# Will compile xdpsock_1.c and xdpsock_2.c, and then link the executable
>> +# xdpsock, based on xdpsock_1.o and xdpsock_2.o
>> +#
>> +# Inherited from scripts/Makefile.host
>
>"Inspired by" or "Derived from" would be probably more appropriate term :)
I will replace with "Derived from", looks better.

>
>> +#
>> +__tprogs := $(sort $(tprogs-y))
>> +
>> +# C code
>> +# Executables compiled from a single .c file
>> +tprog-csingle  := $(foreach m,$(__tprogs), \
>> +                       $(if $($(m)-objs),,$(m)))
>> +
>> +# C executables linked based on several .o files
>> +tprog-cmulti   := $(foreach m,$(__tprogs),\
>> +                       $(if $($(m)-objs),$(m)))
>> +
>> +# Object (.o) files compiled from .c files
>> +tprog-cobjs    := $(sort $(foreach m,$(__tprogs),$($(m)-objs)))
>> +
>> +tprog-csingle  := $(addprefix $(obj)/,$(tprog-csingle))
>> +tprog-cmulti   := $(addprefix $(obj)/,$(tprog-cmulti))
>> +tprog-cobjs    := $(addprefix $(obj)/,$(tprog-cobjs))
>> +
>> +#####
>> +# Handle options to gcc. Support building with separate output directory
>> +
>> +_tprogc_flags   = $(TPROGS_CFLAGS) \
>> +                 $(TPROGCFLAGS_$(basetarget).o)
>> +
>> +# $(objtree)/$(obj) for including generated headers from checkin source files
>> +ifeq ($(KBUILD_EXTMOD),)
>> +ifdef building_out_of_srctree
>> +_tprogc_flags   += -I $(objtree)/$(obj)
>> +endif
>> +endif
>> +
>> +tprogc_flags    = -Wp,-MD,$(depfile) $(_tprogc_flags)
>> +
>> +# Create executable from a single .c file
>> +# tprog-csingle -> Executable
>> +quiet_cmd_tprog-csingle        = CC  $@
>> +      cmd_tprog-csingle        = $(CC) $(tprogc_flags) $(TPROGS_LDFLAGS) -o $@ $< \
>> +               $(TPROGS_LDLIBS) $(TPROGLDLIBS_$(@F))
>> +$(tprog-csingle): $(obj)/%: $(src)/%.c FORCE
>> +       $(call if_changed_dep,tprog-csingle)
>> +
>> +# Link an executable based on list of .o files, all plain c
>> +# tprog-cmulti -> executable
>> +quiet_cmd_tprog-cmulti = LD  $@
>> +      cmd_tprog-cmulti = $(CC) $(tprogc_flags) $(TPROGS_LDFLAGS) -o $@ \
>> +                         $(addprefix $(obj)/,$($(@F)-objs)) \
>> +                         $(TPROGS_LDLIBS) $(TPROGLDLIBS_$(@F))
>> +$(tprog-cmulti): $(tprog-cobjs) FORCE
>> +       $(call if_changed,tprog-cmulti)
>> +$(call multi_depend, $(tprog-cmulti), , -objs)
>> +
>> +# Create .o file from a single .c file
>> +# tprog-cobjs -> .o
>> +quiet_cmd_tprog-cobjs  = CC  $@
>> +      cmd_tprog-cobjs  = $(CC) $(tprogc_flags) -c -o $@ $<
>> +$(tprog-cobjs): $(obj)/%.o: $(src)/%.c FORCE
>> +       $(call if_changed_dep,tprog-cobjs)
>> --
>> 2.17.1
>>
>
>tprogs is quite confusing, but overall looks good to me.
I tend to leave it as tprogs, unless it's going to be progs and agreed with
Yonghong.

It follows logic:
- tprogs for bins
- bpfprogs or bojs or bprogs (could be) for bpf obj

-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH v3 bpf-next 08/14] samples: bpf: makefile: base target programs rules on Makefile.target

[Ivan Khoronzhuk]

On Tue, Sep 17, 2019 at 04:28:01PM -0700, Andrii Nakryiko wrote:
>On Mon, Sep 16, 2019 at 3:58 AM Ivan Khoronzhuk
><ivan.khoronzhuk@linaro.org> wrote:
>>
>
>Please don't prepend "samples: bpf: makefile:" to patches,
>"samples/bpf: " is a typical we've used for BPF samples changes.
Ok.

>
>
>> The main reason for that - HOSTCC and CC have different aims.
>> HOSTCC is used to build programs running on host, that can
>> cross-comple target programs with CC. It was tested for arm and arm64
>> cross compilation, based on linaro toolchain, but should work for
>> others.
>>
>> So, in order to split cross compilation (CC) with host build (HOSTCC),
>> lets base samples on Makefile.target. It allows to cross-compile
>> samples/bpf programs with CC while auxialry tools running on host
>> built with HOSTCC.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>>  samples/bpf/Makefile | 135 ++++++++++++++++++++++---------------------
>>  1 file changed, 69 insertions(+), 66 deletions(-)
>>
>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> index 9d923546e087..1579cc16a1c2 100644
>> --- a/samples/bpf/Makefile
>> +++ b/samples/bpf/Makefile
>> @@ -4,55 +4,53 @@ BPF_SAMPLES_PATH ?= $(abspath $(srctree)/$(src))
>>  TOOLS_PATH := $(BPF_SAMPLES_PATH)/../../tools
>>
>>  # List of programs to build
>> -hostprogs-y := test_lru_dist
[...]
>> -KBUILD_HOSTCFLAGS := $(ARM_ARCH_SELECTOR)
>> +TPROGS_CFLAGS += $(ARM_ARCH_SELECTOR)
>>  endif
>>
>> +TPROGS_LDLIBS := $(KBUILD_HOSTLDLIBS)
>
>Please group TPROGS_LDLIBS definition together with the one below,
>there doesn't seem to be a reason to split them this way.
No. It's used in Makefile.target and should be here, following hostprog logic.

>
>But also, it's kind of weird to use host libraries as cross-compiled
>libraries as well. Is that intentional?
No cross-compile split yet. This patch replace only KBUILD on TPROGS.
It's done in following patches.

>
>> +TPROGS_CFLAGS += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS)
>
>Same here, is it right to use HOSTCFLAGS and HOST_EXTRACFLAGS as a
>base for cross-compiled cflags?
same

[...]

-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH v3 bpf-next 09/14] samples: bpf: makefile: use own flags but not host when cross compile

[Ivan Khoronzhuk]

On Tue, Sep 17, 2019 at 04:42:07PM -0700, Andrii Nakryiko wrote:
>On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk
><ivan.khoronzhuk@linaro.org> wrote:
>>
>> While compile natively, the hosts cflags and ldflags are equal to ones
>> used from HOSTCFLAGS and HOSTLDFLAGS. When cross compiling it should
>> have own, used for target arch. While verification, for arm, arm64 and
>> x86_64 the following flags were used alsways:
>>
>> -Wall
>> -O2
>> -fomit-frame-pointer
>> -Wmissing-prototypes
>> -Wstrict-prototypes
>>
>> So, add them as they were verified and used before adding
>> Makefile.target, but anyway limit it only for cross compile options as
>> for host can be some configurations when another options can be used,
>> So, for host arch samples left all as is, it allows to avoid potential
>> option mistmatches for existent environments.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>>  samples/bpf/Makefile | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> index 1579cc16a1c2..b5c87a8b8b51 100644
>> --- a/samples/bpf/Makefile
>> +++ b/samples/bpf/Makefile
>> @@ -178,8 +178,17 @@ CLANG_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR)
>>  TPROGS_CFLAGS += $(ARM_ARCH_SELECTOR)
>>  endif
>>
>> +ifdef CROSS_COMPILE
>> +TPROGS_CFLAGS += -Wall
>> +TPROGS_CFLAGS += -O2
>
>Specifying one arg per line seems like overkill, put them in one line?
Will combine.

>
>> +TPROGS_CFLAGS += -fomit-frame-pointer
>
>Why this one?
I've explained in commit msg. The logic is to have as much as close options
to have smiliar binaries. As those options are used before for hosts and kinda
cross builds - better follow same way.

>
>> +TPROGS_CFLAGS += -Wmissing-prototypes
>> +TPROGS_CFLAGS += -Wstrict-prototypes
>
>Are these in some way special that we want them in cross-compile mode only?
>
>All of those flags seem useful regardless of cross-compilation or not,
>shouldn't they be common? I'm a bit lost about the intent here...
They are common but split is needed to expose it at least. Also host for
different arches can have some own opts already used that shouldn't be present
for cross, better not mix it for safety.

>
>> +else
>>  TPROGS_LDLIBS := $(KBUILD_HOSTLDLIBS)
>>  TPROGS_CFLAGS += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS)
>> +endif
>> +
>>  TPROGS_CFLAGS += -I$(objtree)/usr/include
>>  TPROGS_CFLAGS += -I$(srctree)/tools/lib/bpf/
>>  TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
>> --
>> 2.17.1
>>

-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH v3 bpf-next 11/14] libbpf: makefile: add C/CXX/LDFLAGS to libbpf.so and test_libpf targets

[Ivan Khoronzhuk]

On Tue, Sep 17, 2019 at 10:19:22PM -0700, Andrii Nakryiko wrote:
>On Mon, Sep 16, 2019 at 4:00 AM Ivan Khoronzhuk
><ivan.khoronzhuk@linaro.org> wrote:
>>
>> In case of LDFLAGS and EXTRA_CC/CXX flags there is no way to pass them
>> correctly to build command, for instance when --sysroot is used or
>> external libraries are used, like -lelf, wich can be absent in
>> toolchain. This can be used for samples/bpf cross-compiling allowing
>> to get elf lib from sysroot.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>>  tools/lib/bpf/Makefile | 11 ++++++++---
>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
>> index c6f94cffe06e..bccfa556ef4e 100644
>> --- a/tools/lib/bpf/Makefile
>> +++ b/tools/lib/bpf/Makefile
>> @@ -94,6 +94,10 @@ else
>>    CFLAGS := -g -Wall
>>  endif
>>
>> +ifdef EXTRA_CXXFLAGS
>> +  CXXFLAGS := $(EXTRA_CXXFLAGS)
>> +endif
>> +
>>  ifeq ($(feature-libelf-mmap), 1)
>>    override CFLAGS += -DHAVE_LIBELF_MMAP_SUPPORT
>>  endif
>> @@ -176,8 +180,9 @@ $(BPF_IN): force elfdep bpfdep
>>  $(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION)
>>
>>  $(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN)
>> -       $(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
>> -                                   -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
>> +       $(QUIET_LINK)$(CC) $(LDFLAGS) \
>> +               --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
>> +               -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
>>         @ln -sf $(@F) $(OUTPUT)libbpf.so
>>         @ln -sf $(@F) $(OUTPUT)libbpf.so.$(LIBBPF_MAJOR_VERSION)
>>
>> @@ -185,7 +190,7 @@ $(OUTPUT)libbpf.a: $(BPF_IN)
>>         $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
>>
>>  $(OUTPUT)test_libbpf: test_libbpf.cpp $(OUTPUT)libbpf.a
>> -       $(QUIET_LINK)$(CXX) $(INCLUDES) $^ -lelf -o $@
>> +       $(QUIET_LINK)$(CXX) $(CXXFLAGS) $(LDFLAGS) $(INCLUDES) $^ -lelf -o $@
>
>Instead of doing ifdef EXTRA_CXXFLAGS bit above, you can just include
>both $(CXXFLAGS) and $(EXTRA_CXXFLAGS), which will do the right thing
>(and is actually recommended my make documentation way to do this).
It's good practice to follow existent style, I've done similar way as for
CFLAGS + EXTRACFLAGS here, didn't want to verify it can impact on
smth else. And my goal is not to correct everything but embed my
functionality, series tool large w/o it.

>
>But actually, there is no need to use C++ compiler here,
>test_libbpf.cpp can just be plain C. Do you mind renaming it to .c and
>using C compiler instead?
Seems like, will try in next v.

>
>>
>>  $(OUTPUT)libbpf.pc:
>>         $(QUIET_GEN)sed -e "s|@PREFIX@|$(prefix)|" \
>> --
>> 2.17.1
>>

-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH v3 bpf-next 13/14] samples: bpf: makefile: add sysroot support

[Ivan Khoronzhuk]

On Tue, Sep 17, 2019 at 10:23:57PM -0700, Andrii Nakryiko wrote:
>On Mon, Sep 16, 2019 at 4:00 AM Ivan Khoronzhuk
><ivan.khoronzhuk@linaro.org> wrote:
>>
>> Basically it only enables that was added by previous couple fixes.
>> Sysroot contains correct libs installed and its headers ofc. Useful
>
>Please, let's not use unnecessary abbreviations/slang. "Of course" is
>not too long and is a proper English, let's stick to it.
>
>> when working with NFC or virtual machine.
>>
>> Usage:
>>
>> clean (on demand)
>>     make ARCH=arm -C samples/bpf clean
>>     make ARCH=arm -C tools clean
>>     make ARCH=arm clean
>>
>> configure and install headers:
>>
>>     make ARCH=arm defconfig
>>     make ARCH=arm headers_install
>>
>> build samples/bpf:
>>     make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- samples/bpf/ \
>>     SYSROOT="path/to/sysroot"
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>>  samples/bpf/Makefile | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> index 133123d4c7d7..57ddf055d6c3 100644
>> --- a/samples/bpf/Makefile
>> +++ b/samples/bpf/Makefile
>> @@ -194,6 +194,11 @@ TPROGS_CFLAGS += -I$(srctree)/tools/lib/
>>  TPROGS_CFLAGS += -I$(srctree)/tools/include
>>  TPROGS_CFLAGS += -I$(srctree)/tools/perf
>>
>> +ifdef SYSROOT
>> +TPROGS_CFLAGS += --sysroot=${SYSROOT}
>> +TPROGS_LDFLAGS := -L${SYSROOT}/usr/lib
>
>Please stay consistent: $() instead of ${}?
Yes, thanks.

>
>> +endif
>> +
>>  EXTRA_CXXFLAGS := $(TPROGS_CFLAGS)
>>
>>  # options not valid for C++
>> --
>> 2.17.1
>>

-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH v3 bpf-next 07/14] samples: bpf: add makefile.target for separate CC target build

[Andrii Nakryiko]

On Wed, Sep 18, 2019 at 3:12 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> On Tue, Sep 17, 2019 at 04:19:40PM -0700, Andrii Nakryiko wrote:
> >On Mon, Sep 16, 2019 at 3:58 AM Ivan Khoronzhuk
> ><ivan.khoronzhuk@linaro.org> wrote:
> >>
> >> The makefile.target is added only and will be used in
> >
> >typo: Makefile
> >
> >> sample/bpf/Makefile later in order to switch cross-compiling on CC
> >
> >on -> to
> >
> >> from HOSTCC environment.
> >>
> >> The HOSTCC is supposed to build binaries and tools running on the host
> >> afterwards, in order to simplify build or so, like "fixdep" or else.
> >> In case of cross compiling "fixdep" is executed on host when the rest
> >> samples should run on target arch. In order to build binaries for
> >> target arch with CC and tools running on host with HOSTCC, lets add
> >> Makefile.target for simplicity, having definition and routines similar
> >> to ones, used in script/Makefile.host. This allows later add
> >> cross-compilation to samples/bpf with minimum changes.
> >>
> >> The tprog stands for target programs built with CC.
> >
> >Why tprog? Could we just use prog: hostprog vs prog.
> Prev. version was with prog, but Yonghong Song found it ambiguous.
> As prog can be bpf also. So, decision was made to follow logic:
> * target prog - non bpf progs
> * bpf prog = bpf prog, that can be later smth similar, providing build options
>   for each bpf object separately.
>

Well, I'm not going to insist, but BPF program is a C function,
compiled BPF .o file is BPF object, so I don't think there is going to
be too much confusion to have progs and hostprogs in Makefile. But I'm
fine with tprog.

> Details here:
> https://lkml.org/lkml/2019/9/13/1037
>
> >
> >>
> >> Makefile.target contains only stuff needed for samples/bpf, potentially
> >> can be reused later and now needed only for unblocking tricky
> >> samples/bpf cross compilation.
> >>
> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> >> ---
> >>  samples/bpf/Makefile.target | 75 +++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 75 insertions(+)
> >>  create mode 100644 samples/bpf/Makefile.target
> >>
> >> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
> >> new file mode 100644
> >> index 000000000000..fb6de63f7d2f
> >> --- /dev/null
> >> +++ b/samples/bpf/Makefile.target
> >> @@ -0,0 +1,75 @@
> >> +# SPDX-License-Identifier: GPL-2.0
> >> +# ==========================================================================
> >> +# Building binaries on the host system
> >> +# Binaries are not used during the compilation of the kernel, and intendent
> >
> >typo: intended
> >
> >> +# to be build for target board, target board can be host ofc. Added to build
> >
> >What's ofc, is it "of course"?
> yes, ofc )

Alright, let's not try to save 5 letters, it's quite confusing.

>
> >
> >> +# binaries to run not on host system.
> >> +#
> >> +# Sample syntax (see Documentation/kbuild/makefiles.rst for reference)
> >> +# tprogs-y := xsk_example
> >> +# Will compile xdpsock_example.c and create an executable named xsk_example
> >
> >You mix references to xsk_example and xdpsock_example, which is very
> >confusing. I'm guessing you meant to use xdpsock_example consistently.
> Oh, yes. Thanks.
>
> >
> >> +#
> >> +# tprogs-y    := xdpsock
> >> +# xdpsock-objs := xdpsock_1.o xdpsock_2.o
> >> +# Will compile xdpsock_1.c and xdpsock_2.c, and then link the executable
> >> +# xdpsock, based on xdpsock_1.o and xdpsock_2.o
> >> +#
> >> +# Inherited from scripts/Makefile.host
> >
> >"Inspired by" or "Derived from" would be probably more appropriate term :)
> I will replace with "Derived from", looks better.
>

sounds good

> >
> >> +#
> >> +__tprogs := $(sort $(tprogs-y))
> >> +
> >> +# C code
> >> +# Executables compiled from a single .c file
> >> +tprog-csingle  := $(foreach m,$(__tprogs), \
> >> +                       $(if $($(m)-objs),,$(m)))
> >> +
> >> +# C executables linked based on several .o files
> >> +tprog-cmulti   := $(foreach m,$(__tprogs),\
> >> +                       $(if $($(m)-objs),$(m)))
> >> +
> >> +# Object (.o) files compiled from .c files
> >> +tprog-cobjs    := $(sort $(foreach m,$(__tprogs),$($(m)-objs)))
> >> +
> >> +tprog-csingle  := $(addprefix $(obj)/,$(tprog-csingle))
> >> +tprog-cmulti   := $(addprefix $(obj)/,$(tprog-cmulti))
> >> +tprog-cobjs    := $(addprefix $(obj)/,$(tprog-cobjs))
> >> +
> >> +#####
> >> +# Handle options to gcc. Support building with separate output directory
> >> +
> >> +_tprogc_flags   = $(TPROGS_CFLAGS) \
> >> +                 $(TPROGCFLAGS_$(basetarget).o)
> >> +
> >> +# $(objtree)/$(obj) for including generated headers from checkin source files
> >> +ifeq ($(KBUILD_EXTMOD),)
> >> +ifdef building_out_of_srctree
> >> +_tprogc_flags   += -I $(objtree)/$(obj)
> >> +endif
> >> +endif
> >> +
> >> +tprogc_flags    = -Wp,-MD,$(depfile) $(_tprogc_flags)
> >> +
> >> +# Create executable from a single .c file
> >> +# tprog-csingle -> Executable
> >> +quiet_cmd_tprog-csingle        = CC  $@
> >> +      cmd_tprog-csingle        = $(CC) $(tprogc_flags) $(TPROGS_LDFLAGS) -o $@ $< \
> >> +               $(TPROGS_LDLIBS) $(TPROGLDLIBS_$(@F))
> >> +$(tprog-csingle): $(obj)/%: $(src)/%.c FORCE
> >> +       $(call if_changed_dep,tprog-csingle)
> >> +
> >> +# Link an executable based on list of .o files, all plain c
> >> +# tprog-cmulti -> executable
> >> +quiet_cmd_tprog-cmulti = LD  $@
> >> +      cmd_tprog-cmulti = $(CC) $(tprogc_flags) $(TPROGS_LDFLAGS) -o $@ \
> >> +                         $(addprefix $(obj)/,$($(@F)-objs)) \
> >> +                         $(TPROGS_LDLIBS) $(TPROGLDLIBS_$(@F))
> >> +$(tprog-cmulti): $(tprog-cobjs) FORCE
> >> +       $(call if_changed,tprog-cmulti)
> >> +$(call multi_depend, $(tprog-cmulti), , -objs)
> >> +
> >> +# Create .o file from a single .c file
> >> +# tprog-cobjs -> .o
> >> +quiet_cmd_tprog-cobjs  = CC  $@
> >> +      cmd_tprog-cobjs  = $(CC) $(tprogc_flags) -c -o $@ $<
> >> +$(tprog-cobjs): $(obj)/%.o: $(src)/%.c FORCE
> >> +       $(call if_changed_dep,tprog-cobjs)
> >> --
> >> 2.17.1
> >>
> >
> >tprogs is quite confusing, but overall looks good to me.
> I tend to leave it as tprogs, unless it's going to be progs and agreed with
> Yonghong.
>
> It follows logic:
> - tprogs for bins
> - bpfprogs or bojs or bprogs (could be) for bpf obj

as mentioned above, we never build "BPF programs", they are always
part of BPF objects. But as I mentioned, I'm fine with sticking to
tprog.

>
> --
> Regards,
> Ivan Khoronzhuk

Re: [PATCH v3 bpf-next 09/14] samples: bpf: makefile: use own flags but not host when cross compile

[Andrii Nakryiko]

On Wed, Sep 18, 2019 at 3:35 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> On Tue, Sep 17, 2019 at 04:42:07PM -0700, Andrii Nakryiko wrote:
> >On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk
> ><ivan.khoronzhuk@linaro.org> wrote:
> >>
> >> While compile natively, the hosts cflags and ldflags are equal to ones
> >> used from HOSTCFLAGS and HOSTLDFLAGS. When cross compiling it should
> >> have own, used for target arch. While verification, for arm, arm64 and
> >> x86_64 the following flags were used alsways:
> >>
> >> -Wall
> >> -O2
> >> -fomit-frame-pointer
> >> -Wmissing-prototypes
> >> -Wstrict-prototypes
> >>
> >> So, add them as they were verified and used before adding
> >> Makefile.target, but anyway limit it only for cross compile options as
> >> for host can be some configurations when another options can be used,
> >> So, for host arch samples left all as is, it allows to avoid potential
> >> option mistmatches for existent environments.
> >>
> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> >> ---
> >>  samples/bpf/Makefile | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >>
> >> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> >> index 1579cc16a1c2..b5c87a8b8b51 100644
> >> --- a/samples/bpf/Makefile
> >> +++ b/samples/bpf/Makefile
> >> @@ -178,8 +178,17 @@ CLANG_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR)
> >>  TPROGS_CFLAGS += $(ARM_ARCH_SELECTOR)
> >>  endif
> >>
> >> +ifdef CROSS_COMPILE
> >> +TPROGS_CFLAGS += -Wall
> >> +TPROGS_CFLAGS += -O2
> >
> >Specifying one arg per line seems like overkill, put them in one line?
> Will combine.
>
> >
> >> +TPROGS_CFLAGS += -fomit-frame-pointer
> >
> >Why this one?
> I've explained in commit msg. The logic is to have as much as close options
> to have smiliar binaries. As those options are used before for hosts and kinda
> cross builds - better follow same way.

I'm just asking why omit frame pointers and make it harder to do stuff
like profiling? What performance benefits are we seeking for in BPF
samples?

>
> >
> >> +TPROGS_CFLAGS += -Wmissing-prototypes
> >> +TPROGS_CFLAGS += -Wstrict-prototypes
> >
> >Are these in some way special that we want them in cross-compile mode only?
> >
> >All of those flags seem useful regardless of cross-compilation or not,
> >shouldn't they be common? I'm a bit lost about the intent here...
> They are common but split is needed to expose it at least. Also host for
> different arches can have some own opts already used that shouldn't be present
> for cross, better not mix it for safety.

We want -Wmissing-prototypes and -Wstrict-prototypes for cross-compile
and non-cross-compile cases, right? So let's specify them as common
set of options, instead of relying on KBUILD_HOSTCFLAGS or
HOST_EXTRACFLAGS to have them. Otherwise we'll be getting extra
warnings for just cross-compile case, which is not good. If you are
worrying about having duplicate -W flags, seems like it's handled by
GCC already, so shouldn't be a problem.

>
> >
> >> +else
> >>  TPROGS_LDLIBS := $(KBUILD_HOSTLDLIBS)
> >>  TPROGS_CFLAGS += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS)
> >> +endif
> >> +
> >>  TPROGS_CFLAGS += -I$(objtree)/usr/include
> >>  TPROGS_CFLAGS += -I$(srctree)/tools/lib/bpf/
> >>  TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
> >> --
> >> 2.17.1
> >>
>
> --
> Regards,
> Ivan Khoronzhuk

Re: [PATCH v3 bpf-next 11/14] libbpf: makefile: add C/CXX/LDFLAGS to libbpf.so and test_libpf targets

[Andrii Nakryiko]

On Wed, Sep 18, 2019 at 4:05 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> On Tue, Sep 17, 2019 at 10:19:22PM -0700, Andrii Nakryiko wrote:
> >On Mon, Sep 16, 2019 at 4:00 AM Ivan Khoronzhuk
> ><ivan.khoronzhuk@linaro.org> wrote:
> >>
> >> In case of LDFLAGS and EXTRA_CC/CXX flags there is no way to pass them
> >> correctly to build command, for instance when --sysroot is used or
> >> external libraries are used, like -lelf, wich can be absent in
> >> toolchain. This can be used for samples/bpf cross-compiling allowing
> >> to get elf lib from sysroot.
> >>
> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> >> ---
> >>  tools/lib/bpf/Makefile | 11 ++++++++---
> >>  1 file changed, 8 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> >> index c6f94cffe06e..bccfa556ef4e 100644
> >> --- a/tools/lib/bpf/Makefile
> >> +++ b/tools/lib/bpf/Makefile
> >> @@ -94,6 +94,10 @@ else
> >>    CFLAGS := -g -Wall
> >>  endif
> >>
> >> +ifdef EXTRA_CXXFLAGS
> >> +  CXXFLAGS := $(EXTRA_CXXFLAGS)
> >> +endif
> >> +
> >>  ifeq ($(feature-libelf-mmap), 1)
> >>    override CFLAGS += -DHAVE_LIBELF_MMAP_SUPPORT
> >>  endif
> >> @@ -176,8 +180,9 @@ $(BPF_IN): force elfdep bpfdep
> >>  $(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION)
> >>
> >>  $(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN)
> >> -       $(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
> >> -                                   -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
> >> +       $(QUIET_LINK)$(CC) $(LDFLAGS) \
> >> +               --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
> >> +               -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
> >>         @ln -sf $(@F) $(OUTPUT)libbpf.so
> >>         @ln -sf $(@F) $(OUTPUT)libbpf.so.$(LIBBPF_MAJOR_VERSION)
> >>
> >> @@ -185,7 +190,7 @@ $(OUTPUT)libbpf.a: $(BPF_IN)
> >>         $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
> >>
> >>  $(OUTPUT)test_libbpf: test_libbpf.cpp $(OUTPUT)libbpf.a
> >> -       $(QUIET_LINK)$(CXX) $(INCLUDES) $^ -lelf -o $@
> >> +       $(QUIET_LINK)$(CXX) $(CXXFLAGS) $(LDFLAGS) $(INCLUDES) $^ -lelf -o $@
> >
> >Instead of doing ifdef EXTRA_CXXFLAGS bit above, you can just include
> >both $(CXXFLAGS) and $(EXTRA_CXXFLAGS), which will do the right thing
> >(and is actually recommended my make documentation way to do this).
> It's good practice to follow existent style, I've done similar way as for
> CFLAGS + EXTRACFLAGS here, didn't want to verify it can impact on
> smth else. And my goal is not to correct everything but embed my
> functionality, series tool large w/o it.

Alright, we'll have to eventually clean up this Makefile. What we do
with EXTRA_CFLAGS is not exactly correct, as in this Makefile
EXTRA_CFLAGS are overriding CFLAGS, instead of extending them, which
doesn't seem correct to me. BTW, bpftool does += instead of :=. All
this is avoided by just keeping CFLAGS and EXTRA_CFLAGS separate and
specifying both of them in $(CC)/$(CLANG) invocations. But feel free
to ignore this for now.


>
> >
> >But actually, there is no need to use C++ compiler here,
> >test_libbpf.cpp can just be plain C. Do you mind renaming it to .c and
> >using C compiler instead?
> Seems like, will try in next v.

Thanks!

>
> >
> >>
> >>  $(OUTPUT)libbpf.pc:
> >>         $(QUIET_GEN)sed -e "s|@PREFIX@|$(prefix)|" \
> >> --
> >> 2.17.1
> >>
>
> --
> Regards,
> Ivan Khoronzhuk

Re: [PATCH v3 bpf-next 09/14] samples: bpf: makefile: use own flags but not host when cross compile

[Ivan Khoronzhuk]

On Wed, Sep 18, 2019 at 02:29:53PM -0700, Andrii Nakryiko wrote:
>On Wed, Sep 18, 2019 at 3:35 AM Ivan Khoronzhuk
><ivan.khoronzhuk@linaro.org> wrote:
>>
>> On Tue, Sep 17, 2019 at 04:42:07PM -0700, Andrii Nakryiko wrote:
>> >On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk
>> ><ivan.khoronzhuk@linaro.org> wrote:
>> >>
>> >> While compile natively, the hosts cflags and ldflags are equal to ones
>> >> used from HOSTCFLAGS and HOSTLDFLAGS. When cross compiling it should
>> >> have own, used for target arch. While verification, for arm, arm64 and
>> >> x86_64 the following flags were used alsways:
>> >>
>> >> -Wall
>> >> -O2
>> >> -fomit-frame-pointer
>> >> -Wmissing-prototypes
>> >> -Wstrict-prototypes
>> >>
>> >> So, add them as they were verified and used before adding
>> >> Makefile.target, but anyway limit it only for cross compile options as
>> >> for host can be some configurations when another options can be used,
>> >> So, for host arch samples left all as is, it allows to avoid potential
>> >> option mistmatches for existent environments.
>> >>
>> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> >> ---
>> >>  samples/bpf/Makefile | 9 +++++++++
>> >>  1 file changed, 9 insertions(+)
>> >>
>> >> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> >> index 1579cc16a1c2..b5c87a8b8b51 100644
>> >> --- a/samples/bpf/Makefile
>> >> +++ b/samples/bpf/Makefile
>> >> @@ -178,8 +178,17 @@ CLANG_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR)
>> >>  TPROGS_CFLAGS += $(ARM_ARCH_SELECTOR)
>> >>  endif
>> >>
>> >> +ifdef CROSS_COMPILE
>> >> +TPROGS_CFLAGS += -Wall
>> >> +TPROGS_CFLAGS += -O2
>> >
>> >Specifying one arg per line seems like overkill, put them in one line?
>> Will combine.
>>
>> >
>> >> +TPROGS_CFLAGS += -fomit-frame-pointer
>> >
>> >Why this one?
>> I've explained in commit msg. The logic is to have as much as close options
>> to have smiliar binaries. As those options are used before for hosts and kinda
>> cross builds - better follow same way.
>
>I'm just asking why omit frame pointers and make it harder to do stuff
>like profiling? What performance benefits are we seeking for in BPF
>samples?
>
>>
>> >
>> >> +TPROGS_CFLAGS += -Wmissing-prototypes
>> >> +TPROGS_CFLAGS += -Wstrict-prototypes
>> >
>> >Are these in some way special that we want them in cross-compile mode only?
>> >
>> >All of those flags seem useful regardless of cross-compilation or not,
>> >shouldn't they be common? I'm a bit lost about the intent here...
>> They are common but split is needed to expose it at least. Also host for
>> different arches can have some own opts already used that shouldn't be present
>> for cross, better not mix it for safety.
>
>We want -Wmissing-prototypes and -Wstrict-prototypes for cross-compile
>and non-cross-compile cases, right? So let's specify them as common
>set of options, instead of relying on KBUILD_HOSTCFLAGS or
>HOST_EXTRACFLAGS to have them. Otherwise we'll be getting extra
>warnings for just cross-compile case, which is not good. If you are
>worrying about having duplicate -W flags, seems like it's handled by
>GCC already, so shouldn't be a problem.

Ok, lets drop omit-frame-pointer.

But then, lets do more radical step and drop
KBUILD_HOSTCFLAGS & HOST_EXTRACFLAG in this patch:

-ifdef CROSS_COMPILE
+TPROGS_CFLAGS += -Wall -O2
+TPROGS_CFLAGS += -Wmissing-prototypes
+TPROGS_CFLAGS += -Wstrict-prototypes
-else
-TPROGS_LDLIBS := $(KBUILD_HOSTLDLIBS)
-TPROGS_CFLAGS += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS)
-endif

At least it allows to use same options always for both, native and cross.

I verified on native x86_64, arm64 and arm and cross for arm and arm64,
but should work for others, at least it can be tuned explicitly and
no need to depend on KBUILD and use "cross" fork here.

-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH v3 bpf-next 09/14] samples: bpf: makefile: use own flags but not host when cross compile

[Andrii Nakryiko]

On Thu, Sep 19, 2019 at 7:18 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> On Wed, Sep 18, 2019 at 02:29:53PM -0700, Andrii Nakryiko wrote:
> >On Wed, Sep 18, 2019 at 3:35 AM Ivan Khoronzhuk
> ><ivan.khoronzhuk@linaro.org> wrote:
> >>
> >> On Tue, Sep 17, 2019 at 04:42:07PM -0700, Andrii Nakryiko wrote:
> >> >On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk
> >> ><ivan.khoronzhuk@linaro.org> wrote:
> >> >>
> >> >> While compile natively, the hosts cflags and ldflags are equal to ones
> >> >> used from HOSTCFLAGS and HOSTLDFLAGS. When cross compiling it should
> >> >> have own, used for target arch. While verification, for arm, arm64 and
> >> >> x86_64 the following flags were used alsways:
> >> >>
> >> >> -Wall
> >> >> -O2
> >> >> -fomit-frame-pointer
> >> >> -Wmissing-prototypes
> >> >> -Wstrict-prototypes
> >> >>
> >> >> So, add them as they were verified and used before adding
> >> >> Makefile.target, but anyway limit it only for cross compile options as
> >> >> for host can be some configurations when another options can be used,
> >> >> So, for host arch samples left all as is, it allows to avoid potential
> >> >> option mistmatches for existent environments.
> >> >>
> >> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> >> >> ---
> >> >>  samples/bpf/Makefile | 9 +++++++++
> >> >>  1 file changed, 9 insertions(+)
> >> >>
> >> >> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> >> >> index 1579cc16a1c2..b5c87a8b8b51 100644
> >> >> --- a/samples/bpf/Makefile
> >> >> +++ b/samples/bpf/Makefile
> >> >> @@ -178,8 +178,17 @@ CLANG_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR)
> >> >>  TPROGS_CFLAGS += $(ARM_ARCH_SELECTOR)
> >> >>  endif
> >> >>
> >> >> +ifdef CROSS_COMPILE
> >> >> +TPROGS_CFLAGS += -Wall
> >> >> +TPROGS_CFLAGS += -O2
> >> >
> >> >Specifying one arg per line seems like overkill, put them in one line?
> >> Will combine.
> >>
> >> >
> >> >> +TPROGS_CFLAGS += -fomit-frame-pointer
> >> >
> >> >Why this one?
> >> I've explained in commit msg. The logic is to have as much as close options
> >> to have smiliar binaries. As those options are used before for hosts and kinda
> >> cross builds - better follow same way.
> >
> >I'm just asking why omit frame pointers and make it harder to do stuff
> >like profiling? What performance benefits are we seeking for in BPF
> >samples?
> >
> >>
> >> >
> >> >> +TPROGS_CFLAGS += -Wmissing-prototypes
> >> >> +TPROGS_CFLAGS += -Wstrict-prototypes
> >> >
> >> >Are these in some way special that we want them in cross-compile mode only?
> >> >
> >> >All of those flags seem useful regardless of cross-compilation or not,
> >> >shouldn't they be common? I'm a bit lost about the intent here...
> >> They are common but split is needed to expose it at least. Also host for
> >> different arches can have some own opts already used that shouldn't be present
> >> for cross, better not mix it for safety.
> >
> >We want -Wmissing-prototypes and -Wstrict-prototypes for cross-compile
> >and non-cross-compile cases, right? So let's specify them as common
> >set of options, instead of relying on KBUILD_HOSTCFLAGS or
> >HOST_EXTRACFLAGS to have them. Otherwise we'll be getting extra
> >warnings for just cross-compile case, which is not good. If you are
> >worrying about having duplicate -W flags, seems like it's handled by
> >GCC already, so shouldn't be a problem.
>
> Ok, lets drop omit-frame-pointer.
>
> But then, lets do more radical step and drop
> KBUILD_HOSTCFLAGS & HOST_EXTRACFLAG in this patch:

Yeah, let's do this, if you confirmed that everything still works (and
I don't see a reason why it shouldn't). Thanks.

>
> -ifdef CROSS_COMPILE
> +TPROGS_CFLAGS += -Wall -O2
> +TPROGS_CFLAGS += -Wmissing-prototypes
> +TPROGS_CFLAGS += -Wstrict-prototypes
> -else
> -TPROGS_LDLIBS := $(KBUILD_HOSTLDLIBS)
> -TPROGS_CFLAGS += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS)
> -endif
>
> At least it allows to use same options always for both, native and cross.
>
> I verified on native x86_64, arm64 and arm and cross for arm and arm64,
> but should work for others, at least it can be tuned explicitly and
> no need to depend on KBUILD and use "cross" fork here.

Yep, I like it.

>
> --
> Regards,
> Ivan Khoronzhuk