All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] samples/bpf: Fix build out of source tree
@ 2023-07-06 15:25 ` Anh Tuan Phan
  0 siblings, 0 replies; 24+ messages in thread
From: Anh Tuan Phan @ 2023-07-06 15:25 UTC (permalink / raw)
  To: bpf, ast, daniel, andrii, martin.lau, linux-kernel-mentees

This commit fixes a few compilation issues when building out of source
tree. The command that I used to build samples/bpf:

export KBUILD_OUTPUT=/tmp
make V=1 M=samples/bpf

The compilation failed since it tried to find the header files in the
wrong places between output directory and source tree directory

Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
---
 samples/bpf/Makefile        | 8 ++++----
 samples/bpf/Makefile.target | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 615f24ebc49c..32469aaa82d5 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
 # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
 # TPROGS_CFLAGS causes conflicts
 XDP_SAMPLE_CFLAGS += -Wall -O2 \
-		     -I$(src)/../../tools/include \
+		     -I$(srctree)/tools/include \
 		     -I$(src)/../../tools/include/uapi \
 		     -I$(LIBBPF_INCLUDE) \
-		     -I$(src)/../../tools/testing/selftests/bpf
+		     -I$(srctree)/tools/testing/selftests/bpf

 $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
 $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
@@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
 $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
$(src)/xdp_sample_shared.h
 	@echo "  CLANG-BPF " $@
 	$(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
-		-Wno-compare-distinct-pointer-types -I$(srctree)/include \
+		-Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
 		-I$(srctree)/samples/bpf -I$(srctree)/tools/include \
 		-I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
 		-c $(filter %.bpf.c,$^) -o $@
@@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
xdp_sample.bpf.o

 LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
skel,$(LINKED_SKELS),$($(skel)-deps)))

-BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
+BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
 BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
 BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))

diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
index 7621f55e2947..86a454cfb080 100644
--- a/samples/bpf/Makefile.target
+++ b/samples/bpf/Makefile.target
@@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
                  $(TPROGCFLAGS_$(basetarget).o)

 # $(objtree)/$(obj) for including generated headers from checkin source
files
-ifeq ($(KBUILD_EXTMOD),)
+ifneq ($(KBUILD_EXTMOD),)
 ifdef building_out_of_srctree
 _tprogc_flags   += -I $(objtree)/$(obj)
 endif
-- 
2.34.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v1] samples/bpf: Fix build out of source tree
@ 2023-07-06 15:25 ` Anh Tuan Phan
  0 siblings, 0 replies; 24+ messages in thread
From: Anh Tuan Phan @ 2023-07-06 15:25 UTC (permalink / raw)
  To: bpf, ast, daniel, andrii, martin.lau, linux-kernel-mentees

This commit fixes a few compilation issues when building out of source
tree. The command that I used to build samples/bpf:

export KBUILD_OUTPUT=/tmp
make V=1 M=samples/bpf

The compilation failed since it tried to find the header files in the
wrong places between output directory and source tree directory

Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
---
 samples/bpf/Makefile        | 8 ++++----
 samples/bpf/Makefile.target | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 615f24ebc49c..32469aaa82d5 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
 # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
 # TPROGS_CFLAGS causes conflicts
 XDP_SAMPLE_CFLAGS += -Wall -O2 \
-		     -I$(src)/../../tools/include \
+		     -I$(srctree)/tools/include \
 		     -I$(src)/../../tools/include/uapi \
 		     -I$(LIBBPF_INCLUDE) \
-		     -I$(src)/../../tools/testing/selftests/bpf
+		     -I$(srctree)/tools/testing/selftests/bpf

 $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
 $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
@@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
 $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
$(src)/xdp_sample_shared.h
 	@echo "  CLANG-BPF " $@
 	$(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
-		-Wno-compare-distinct-pointer-types -I$(srctree)/include \
+		-Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
 		-I$(srctree)/samples/bpf -I$(srctree)/tools/include \
 		-I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
 		-c $(filter %.bpf.c,$^) -o $@
@@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
xdp_sample.bpf.o

 LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
skel,$(LINKED_SKELS),$($(skel)-deps)))

-BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
+BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
 BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
 BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))

diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
index 7621f55e2947..86a454cfb080 100644
--- a/samples/bpf/Makefile.target
+++ b/samples/bpf/Makefile.target
@@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
                  $(TPROGCFLAGS_$(basetarget).o)

 # $(objtree)/$(obj) for including generated headers from checkin source
files
-ifeq ($(KBUILD_EXTMOD),)
+ifneq ($(KBUILD_EXTMOD),)
 ifdef building_out_of_srctree
 _tprogc_flags   += -I $(objtree)/$(obj)
 endif
-- 
2.34.1
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* Re: [PATCH v1] samples/bpf: Fix build out of source tree
  2023-07-06 15:25 ` Anh Tuan Phan
@ 2023-07-06 18:09   ` Stanislav Fomichev via Linux-kernel-mentees
  -1 siblings, 0 replies; 24+ messages in thread
From: Stanislav Fomichev @ 2023-07-06 18:09 UTC (permalink / raw)
  To: Anh Tuan Phan; +Cc: bpf, ast, daniel, andrii, martin.lau, linux-kernel-mentees

On 07/06, Anh Tuan Phan wrote:
> This commit fixes a few compilation issues when building out of source
> tree. The command that I used to build samples/bpf:
> 
> export KBUILD_OUTPUT=/tmp
> make V=1 M=samples/bpf
> 
> The compilation failed since it tried to find the header files in the
> wrong places between output directory and source tree directory
> 
> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
> ---
>  samples/bpf/Makefile        | 8 ++++----
>  samples/bpf/Makefile.target | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 615f24ebc49c..32469aaa82d5 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
>  # TPROGS_CFLAGS causes conflicts
>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
> -		     -I$(src)/../../tools/include \
> +		     -I$(srctree)/tools/include \

[..]

>  		     -I$(src)/../../tools/include/uapi \

Does this $(src) need to be changed as well?


>  		     -I$(LIBBPF_INCLUDE) \
> -		     -I$(src)/../../tools/testing/selftests/bpf
> +		     -I$(srctree)/tools/testing/selftests/bpf
> 
>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
> $(src)/xdp_sample_shared.h
>  	@echo "  CLANG-BPF " $@
>  	$(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
> -		-Wno-compare-distinct-pointer-types -I$(srctree)/include \
> +		-Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
>  		-I$(srctree)/samples/bpf -I$(srctree)/tools/include \
>  		-I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
>  		-c $(filter %.bpf.c,$^) -o $@
> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
> xdp_sample.bpf.o
> 
>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
> skel,$(LINKED_SKELS),$($(skel)-deps)))
> 
> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
> 
> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
> index 7621f55e2947..86a454cfb080 100644
> --- a/samples/bpf/Makefile.target
> +++ b/samples/bpf/Makefile.target
> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
>                   $(TPROGCFLAGS_$(basetarget).o)
> 
>  # $(objtree)/$(obj) for including generated headers from checkin source
> files

[..]

> -ifeq ($(KBUILD_EXTMOD),)
> +ifneq ($(KBUILD_EXTMOD),)

This parts seems to be copy-pasted all over the place in its 'ifeq'
form. What is it doing and why is it needed?

>  ifdef building_out_of_srctree
>  _tprogc_flags   += -I $(objtree)/$(obj)
>  endif
> -- 
> 2.34.1

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v1] samples/bpf: Fix build out of source tree
@ 2023-07-06 18:09   ` Stanislav Fomichev via Linux-kernel-mentees
  0 siblings, 0 replies; 24+ messages in thread
From: Stanislav Fomichev via Linux-kernel-mentees @ 2023-07-06 18:09 UTC (permalink / raw)
  To: Anh Tuan Phan; +Cc: daniel, martin.lau, ast, andrii, bpf, linux-kernel-mentees

On 07/06, Anh Tuan Phan wrote:
> This commit fixes a few compilation issues when building out of source
> tree. The command that I used to build samples/bpf:
> 
> export KBUILD_OUTPUT=/tmp
> make V=1 M=samples/bpf
> 
> The compilation failed since it tried to find the header files in the
> wrong places between output directory and source tree directory
> 
> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
> ---
>  samples/bpf/Makefile        | 8 ++++----
>  samples/bpf/Makefile.target | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 615f24ebc49c..32469aaa82d5 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
>  # TPROGS_CFLAGS causes conflicts
>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
> -		     -I$(src)/../../tools/include \
> +		     -I$(srctree)/tools/include \

[..]

>  		     -I$(src)/../../tools/include/uapi \

Does this $(src) need to be changed as well?


>  		     -I$(LIBBPF_INCLUDE) \
> -		     -I$(src)/../../tools/testing/selftests/bpf
> +		     -I$(srctree)/tools/testing/selftests/bpf
> 
>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
> $(src)/xdp_sample_shared.h
>  	@echo "  CLANG-BPF " $@
>  	$(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
> -		-Wno-compare-distinct-pointer-types -I$(srctree)/include \
> +		-Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
>  		-I$(srctree)/samples/bpf -I$(srctree)/tools/include \
>  		-I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
>  		-c $(filter %.bpf.c,$^) -o $@
> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
> xdp_sample.bpf.o
> 
>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
> skel,$(LINKED_SKELS),$($(skel)-deps)))
> 
> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
> 
> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
> index 7621f55e2947..86a454cfb080 100644
> --- a/samples/bpf/Makefile.target
> +++ b/samples/bpf/Makefile.target
> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
>                   $(TPROGCFLAGS_$(basetarget).o)
> 
>  # $(objtree)/$(obj) for including generated headers from checkin source
> files

[..]

> -ifeq ($(KBUILD_EXTMOD),)
> +ifneq ($(KBUILD_EXTMOD),)

This parts seems to be copy-pasted all over the place in its 'ifeq'
form. What is it doing and why is it needed?

>  ifdef building_out_of_srctree
>  _tprogc_flags   += -I $(objtree)/$(obj)
>  endif
> -- 
> 2.34.1
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v1] samples/bpf: Fix build out of source tree
  2023-07-06 18:09   ` Stanislav Fomichev via Linux-kernel-mentees
@ 2023-07-07  1:10     ` Anh Tuan Phan
  -1 siblings, 0 replies; 24+ messages in thread
From: Anh Tuan Phan @ 2023-07-07  1:10 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: bpf, ast, daniel, andrii, martin.lau, linux-kernel-mentees



On 7/7/23 01:09, Stanislav Fomichev wrote:
> On 07/06, Anh Tuan Phan wrote:
>> This commit fixes a few compilation issues when building out of source
>> tree. The command that I used to build samples/bpf:
>>
>> export KBUILD_OUTPUT=/tmp
>> make V=1 M=samples/bpf
>>
>> The compilation failed since it tried to find the header files in the
>> wrong places between output directory and source tree directory
>>
>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
>> ---
>>  samples/bpf/Makefile        | 8 ++++----
>>  samples/bpf/Makefile.target | 2 +-
>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> index 615f24ebc49c..32469aaa82d5 100644
>> --- a/samples/bpf/Makefile
>> +++ b/samples/bpf/Makefile
>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
>>  # TPROGS_CFLAGS causes conflicts
>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
>> -		     -I$(src)/../../tools/include \
>> +		     -I$(srctree)/tools/include \
> 
> [..]
> 
>>  		     -I$(src)/../../tools/include/uapi \
> 
> Does this $(src) need to be changed as well?

I think this line doesn't affect the build. I removed it and it still
compiles (after "make -C samples/bpf clean"). I guess xdp_sample_user.c
doesn't include any file in tools/include/uapi. Am I missing something
or should I remove this line?

> 
> 
>>  		     -I$(LIBBPF_INCLUDE) \
>> -		     -I$(src)/../../tools/testing/selftests/bpf
>> +		     -I$(srctree)/tools/testing/selftests/bpf
>>
>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
>> $(src)/xdp_sample_shared.h
>>  	@echo "  CLANG-BPF " $@
>>  	$(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
>> -		-Wno-compare-distinct-pointer-types -I$(srctree)/include \
>> +		-Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
>>  		-I$(srctree)/samples/bpf -I$(srctree)/tools/include \
>>  		-I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
>>  		-c $(filter %.bpf.c,$^) -o $@
>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
>> xdp_sample.bpf.o
>>
>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
>> skel,$(LINKED_SKELS),$($(skel)-deps)))
>>
>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
>>
>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
>> index 7621f55e2947..86a454cfb080 100644
>> --- a/samples/bpf/Makefile.target
>> +++ b/samples/bpf/Makefile.target
>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
>>                   $(TPROGCFLAGS_$(basetarget).o)
>>
>>  # $(objtree)/$(obj) for including generated headers from checkin source
>> files
> 
> [..]
> 
>> -ifeq ($(KBUILD_EXTMOD),)
>> +ifneq ($(KBUILD_EXTMOD),)
> 
> This parts seems to be copy-pasted all over the place in its 'ifeq'
> form. What is it doing and why is it needed?
> 
>>  ifdef building_out_of_srctree
>>  _tprogc_flags   += -I $(objtree)/$(obj)
>>  endif
>> -- 
>> 2.34.1

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v1] samples/bpf: Fix build out of source tree
@ 2023-07-07  1:10     ` Anh Tuan Phan
  0 siblings, 0 replies; 24+ messages in thread
From: Anh Tuan Phan @ 2023-07-07  1:10 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: daniel, martin.lau, ast, andrii, bpf, linux-kernel-mentees



On 7/7/23 01:09, Stanislav Fomichev wrote:
> On 07/06, Anh Tuan Phan wrote:
>> This commit fixes a few compilation issues when building out of source
>> tree. The command that I used to build samples/bpf:
>>
>> export KBUILD_OUTPUT=/tmp
>> make V=1 M=samples/bpf
>>
>> The compilation failed since it tried to find the header files in the
>> wrong places between output directory and source tree directory
>>
>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
>> ---
>>  samples/bpf/Makefile        | 8 ++++----
>>  samples/bpf/Makefile.target | 2 +-
>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> index 615f24ebc49c..32469aaa82d5 100644
>> --- a/samples/bpf/Makefile
>> +++ b/samples/bpf/Makefile
>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
>>  # TPROGS_CFLAGS causes conflicts
>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
>> -		     -I$(src)/../../tools/include \
>> +		     -I$(srctree)/tools/include \
> 
> [..]
> 
>>  		     -I$(src)/../../tools/include/uapi \
> 
> Does this $(src) need to be changed as well?

I think this line doesn't affect the build. I removed it and it still
compiles (after "make -C samples/bpf clean"). I guess xdp_sample_user.c
doesn't include any file in tools/include/uapi. Am I missing something
or should I remove this line?

> 
> 
>>  		     -I$(LIBBPF_INCLUDE) \
>> -		     -I$(src)/../../tools/testing/selftests/bpf
>> +		     -I$(srctree)/tools/testing/selftests/bpf
>>
>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
>> $(src)/xdp_sample_shared.h
>>  	@echo "  CLANG-BPF " $@
>>  	$(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
>> -		-Wno-compare-distinct-pointer-types -I$(srctree)/include \
>> +		-Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
>>  		-I$(srctree)/samples/bpf -I$(srctree)/tools/include \
>>  		-I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
>>  		-c $(filter %.bpf.c,$^) -o $@
>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
>> xdp_sample.bpf.o
>>
>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
>> skel,$(LINKED_SKELS),$($(skel)-deps)))
>>
>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
>>
>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
>> index 7621f55e2947..86a454cfb080 100644
>> --- a/samples/bpf/Makefile.target
>> +++ b/samples/bpf/Makefile.target
>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
>>                   $(TPROGCFLAGS_$(basetarget).o)
>>
>>  # $(objtree)/$(obj) for including generated headers from checkin source
>> files
> 
> [..]
> 
>> -ifeq ($(KBUILD_EXTMOD),)
>> +ifneq ($(KBUILD_EXTMOD),)
> 
> This parts seems to be copy-pasted all over the place in its 'ifeq'
> form. What is it doing and why is it needed?
> 
>>  ifdef building_out_of_srctree
>>  _tprogc_flags   += -I $(objtree)/$(obj)
>>  endif
>> -- 
>> 2.34.1
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v1] samples/bpf: Fix build out of source tree
  2023-07-07  1:10     ` Anh Tuan Phan
@ 2023-07-07 16:53       ` Stanislav Fomichev via Linux-kernel-mentees
  -1 siblings, 0 replies; 24+ messages in thread
From: Stanislav Fomichev @ 2023-07-07 16:53 UTC (permalink / raw)
  To: Anh Tuan Phan; +Cc: bpf, ast, daniel, andrii, martin.lau, linux-kernel-mentees

On 07/07, Anh Tuan Phan wrote:
> 
> 
> On 7/7/23 01:09, Stanislav Fomichev wrote:
> > On 07/06, Anh Tuan Phan wrote:
> >> This commit fixes a few compilation issues when building out of source
> >> tree. The command that I used to build samples/bpf:
> >>
> >> export KBUILD_OUTPUT=/tmp
> >> make V=1 M=samples/bpf
> >>
> >> The compilation failed since it tried to find the header files in the
> >> wrong places between output directory and source tree directory
> >>
> >> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
> >> ---
> >>  samples/bpf/Makefile        | 8 ++++----
> >>  samples/bpf/Makefile.target | 2 +-
> >>  2 files changed, 5 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> >> index 615f24ebc49c..32469aaa82d5 100644
> >> --- a/samples/bpf/Makefile
> >> +++ b/samples/bpf/Makefile
> >> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
> >>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
> >>  # TPROGS_CFLAGS causes conflicts
> >>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
> >> -		     -I$(src)/../../tools/include \
> >> +		     -I$(srctree)/tools/include \
> > 
> > [..]
> > 
> >>  		     -I$(src)/../../tools/include/uapi \
> > 
> > Does this $(src) need to be changed as well?
> 
> I think this line doesn't affect the build. I removed it and it still
> compiles (after "make -C samples/bpf clean"). I guess xdp_sample_user.c
> doesn't include any file in tools/include/uapi. Am I missing something
> or should I remove this line?

You might have these headers installed on your system already if
it compiles without this part. So I'd keep this part but do
s/src/srctree/ (and remove ../..).

> > 
> > 
> >>  		     -I$(LIBBPF_INCLUDE) \
> >> -		     -I$(src)/../../tools/testing/selftests/bpf
> >> +		     -I$(srctree)/tools/testing/selftests/bpf
> >>
> >>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
> >>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
> >> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
> >>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
> >> $(src)/xdp_sample_shared.h
> >>  	@echo "  CLANG-BPF " $@
> >>  	$(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
> >> -		-Wno-compare-distinct-pointer-types -I$(srctree)/include \
> >> +		-Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
> >>  		-I$(srctree)/samples/bpf -I$(srctree)/tools/include \
> >>  		-I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
> >>  		-c $(filter %.bpf.c,$^) -o $@
> >> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
> >> xdp_sample.bpf.o
> >>
> >>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
> >> skel,$(LINKED_SKELS),$($(skel)-deps)))
> >>
> >> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
> >> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
> >>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
> >>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
> >>
> >> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
> >> index 7621f55e2947..86a454cfb080 100644
> >> --- a/samples/bpf/Makefile.target
> >> +++ b/samples/bpf/Makefile.target
> >> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
> >>                   $(TPROGCFLAGS_$(basetarget).o)
> >>
> >>  # $(objtree)/$(obj) for including generated headers from checkin source
> >> files
> > 
> > [..]
> > 
> >> -ifeq ($(KBUILD_EXTMOD),)
> >> +ifneq ($(KBUILD_EXTMOD),)
> > 
> > This parts seems to be copy-pasted all over the place in its 'ifeq'
> > form. What is it doing and why is it needed?
> > 
> >>  ifdef building_out_of_srctree
> >>  _tprogc_flags   += -I $(objtree)/$(obj)
> >>  endif
> >> -- 
> >> 2.34.1

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v1] samples/bpf: Fix build out of source tree
@ 2023-07-07 16:53       ` Stanislav Fomichev via Linux-kernel-mentees
  0 siblings, 0 replies; 24+ messages in thread
From: Stanislav Fomichev via Linux-kernel-mentees @ 2023-07-07 16:53 UTC (permalink / raw)
  To: Anh Tuan Phan; +Cc: daniel, martin.lau, ast, andrii, bpf, linux-kernel-mentees

On 07/07, Anh Tuan Phan wrote:
> 
> 
> On 7/7/23 01:09, Stanislav Fomichev wrote:
> > On 07/06, Anh Tuan Phan wrote:
> >> This commit fixes a few compilation issues when building out of source
> >> tree. The command that I used to build samples/bpf:
> >>
> >> export KBUILD_OUTPUT=/tmp
> >> make V=1 M=samples/bpf
> >>
> >> The compilation failed since it tried to find the header files in the
> >> wrong places between output directory and source tree directory
> >>
> >> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
> >> ---
> >>  samples/bpf/Makefile        | 8 ++++----
> >>  samples/bpf/Makefile.target | 2 +-
> >>  2 files changed, 5 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> >> index 615f24ebc49c..32469aaa82d5 100644
> >> --- a/samples/bpf/Makefile
> >> +++ b/samples/bpf/Makefile
> >> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
> >>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
> >>  # TPROGS_CFLAGS causes conflicts
> >>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
> >> -		     -I$(src)/../../tools/include \
> >> +		     -I$(srctree)/tools/include \
> > 
> > [..]
> > 
> >>  		     -I$(src)/../../tools/include/uapi \
> > 
> > Does this $(src) need to be changed as well?
> 
> I think this line doesn't affect the build. I removed it and it still
> compiles (after "make -C samples/bpf clean"). I guess xdp_sample_user.c
> doesn't include any file in tools/include/uapi. Am I missing something
> or should I remove this line?

You might have these headers installed on your system already if
it compiles without this part. So I'd keep this part but do
s/src/srctree/ (and remove ../..).

> > 
> > 
> >>  		     -I$(LIBBPF_INCLUDE) \
> >> -		     -I$(src)/../../tools/testing/selftests/bpf
> >> +		     -I$(srctree)/tools/testing/selftests/bpf
> >>
> >>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
> >>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
> >> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
> >>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
> >> $(src)/xdp_sample_shared.h
> >>  	@echo "  CLANG-BPF " $@
> >>  	$(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
> >> -		-Wno-compare-distinct-pointer-types -I$(srctree)/include \
> >> +		-Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
> >>  		-I$(srctree)/samples/bpf -I$(srctree)/tools/include \
> >>  		-I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
> >>  		-c $(filter %.bpf.c,$^) -o $@
> >> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
> >> xdp_sample.bpf.o
> >>
> >>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
> >> skel,$(LINKED_SKELS),$($(skel)-deps)))
> >>
> >> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
> >> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
> >>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
> >>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
> >>
> >> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
> >> index 7621f55e2947..86a454cfb080 100644
> >> --- a/samples/bpf/Makefile.target
> >> +++ b/samples/bpf/Makefile.target
> >> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
> >>                   $(TPROGCFLAGS_$(basetarget).o)
> >>
> >>  # $(objtree)/$(obj) for including generated headers from checkin source
> >> files
> > 
> > [..]
> > 
> >> -ifeq ($(KBUILD_EXTMOD),)
> >> +ifneq ($(KBUILD_EXTMOD),)
> > 
> > This parts seems to be copy-pasted all over the place in its 'ifeq'
> > form. What is it doing and why is it needed?
> > 
> >>  ifdef building_out_of_srctree
> >>  _tprogc_flags   += -I $(objtree)/$(obj)
> >>  endif
> >> -- 
> >> 2.34.1
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v1] samples/bpf: Fix build out of source tree
  2023-07-06 18:09   ` Stanislav Fomichev via Linux-kernel-mentees
@ 2023-07-09  5:12     ` Anh Tuan Phan
  -1 siblings, 0 replies; 24+ messages in thread
From: Anh Tuan Phan @ 2023-07-09  5:12 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: bpf, ast, daniel, andrii, martin.lau, linux-kernel-mentees

Adding one more discussion around your previous question that I missed.

On 7/7/23 01:09, Stanislav Fomichev wrote:
> On 07/06, Anh Tuan Phan wrote:
>> This commit fixes a few compilation issues when building out of source
>> tree. The command that I used to build samples/bpf:
>>
>> export KBUILD_OUTPUT=/tmp
>> make V=1 M=samples/bpf
>>
>> The compilation failed since it tried to find the header files in the
>> wrong places between output directory and source tree directory
>>
>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
>> ---
>>  samples/bpf/Makefile        | 8 ++++----
>>  samples/bpf/Makefile.target | 2 +-
>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> index 615f24ebc49c..32469aaa82d5 100644
>> --- a/samples/bpf/Makefile
>> +++ b/samples/bpf/Makefile
>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
>>  # TPROGS_CFLAGS causes conflicts
>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
>> -		     -I$(src)/../../tools/include \
>> +		     -I$(srctree)/tools/include \
> 
> [..]
> 
>>  		     -I$(src)/../../tools/include/uapi \
> 
> Does this $(src) need to be changed as well?
> 
> 
>>  		     -I$(LIBBPF_INCLUDE) \
>> -		     -I$(src)/../../tools/testing/selftests/bpf
>> +		     -I$(srctree)/tools/testing/selftests/bpf
>>
>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
>> $(src)/xdp_sample_shared.h
>>  	@echo "  CLANG-BPF " $@
>>  	$(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
>> -		-Wno-compare-distinct-pointer-types -I$(srctree)/include \
>> +		-Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
>>  		-I$(srctree)/samples/bpf -I$(srctree)/tools/include \
>>  		-I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
>>  		-c $(filter %.bpf.c,$^) -o $@
>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
>> xdp_sample.bpf.o
>>
>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
>> skel,$(LINKED_SKELS),$($(skel)-deps)))
>>
>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
>>
>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
>> index 7621f55e2947..86a454cfb080 100644
>> --- a/samples/bpf/Makefile.target
>> +++ b/samples/bpf/Makefile.target
>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
>>                   $(TPROGCFLAGS_$(basetarget).o)
>>
>>  # $(objtree)/$(obj) for including generated headers from checkin source
>> files
> 
> [..]
> 
>> -ifeq ($(KBUILD_EXTMOD),)
>> +ifneq ($(KBUILD_EXTMOD),)
> 
> This parts seems to be copy-pasted all over the place in its 'ifeq'
> form. What is it doing and why is it needed?

Sorry for missing this question. As I understand KBUILD_EXTMOD
corresponding to external module. In my case, it's $M=samples/bpf. I
don't really know why it's needed here.

> 
>>  ifdef building_out_of_srctree
>>  _tprogc_flags   += -I $(objtree)/$(obj)
>>  endif
>> -- 
>> 2.34.1

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v1] samples/bpf: Fix build out of source tree
@ 2023-07-09  5:12     ` Anh Tuan Phan
  0 siblings, 0 replies; 24+ messages in thread
From: Anh Tuan Phan @ 2023-07-09  5:12 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: daniel, martin.lau, ast, andrii, bpf, linux-kernel-mentees

Adding one more discussion around your previous question that I missed.

On 7/7/23 01:09, Stanislav Fomichev wrote:
> On 07/06, Anh Tuan Phan wrote:
>> This commit fixes a few compilation issues when building out of source
>> tree. The command that I used to build samples/bpf:
>>
>> export KBUILD_OUTPUT=/tmp
>> make V=1 M=samples/bpf
>>
>> The compilation failed since it tried to find the header files in the
>> wrong places between output directory and source tree directory
>>
>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
>> ---
>>  samples/bpf/Makefile        | 8 ++++----
>>  samples/bpf/Makefile.target | 2 +-
>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> index 615f24ebc49c..32469aaa82d5 100644
>> --- a/samples/bpf/Makefile
>> +++ b/samples/bpf/Makefile
>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
>>  # TPROGS_CFLAGS causes conflicts
>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
>> -		     -I$(src)/../../tools/include \
>> +		     -I$(srctree)/tools/include \
> 
> [..]
> 
>>  		     -I$(src)/../../tools/include/uapi \
> 
> Does this $(src) need to be changed as well?
> 
> 
>>  		     -I$(LIBBPF_INCLUDE) \
>> -		     -I$(src)/../../tools/testing/selftests/bpf
>> +		     -I$(srctree)/tools/testing/selftests/bpf
>>
>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
>> $(src)/xdp_sample_shared.h
>>  	@echo "  CLANG-BPF " $@
>>  	$(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
>> -		-Wno-compare-distinct-pointer-types -I$(srctree)/include \
>> +		-Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
>>  		-I$(srctree)/samples/bpf -I$(srctree)/tools/include \
>>  		-I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
>>  		-c $(filter %.bpf.c,$^) -o $@
>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
>> xdp_sample.bpf.o
>>
>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
>> skel,$(LINKED_SKELS),$($(skel)-deps)))
>>
>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
>>
>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
>> index 7621f55e2947..86a454cfb080 100644
>> --- a/samples/bpf/Makefile.target
>> +++ b/samples/bpf/Makefile.target
>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
>>                   $(TPROGCFLAGS_$(basetarget).o)
>>
>>  # $(objtree)/$(obj) for including generated headers from checkin source
>> files
> 
> [..]
> 
>> -ifeq ($(KBUILD_EXTMOD),)
>> +ifneq ($(KBUILD_EXTMOD),)
> 
> This parts seems to be copy-pasted all over the place in its 'ifeq'
> form. What is it doing and why is it needed?

Sorry for missing this question. As I understand KBUILD_EXTMOD
corresponding to external module. In my case, it's $M=samples/bpf. I
don't really know why it's needed here.

> 
>>  ifdef building_out_of_srctree
>>  _tprogc_flags   += -I $(objtree)/$(obj)
>>  endif
>> -- 
>> 2.34.1
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v1] samples/bpf: Fix build out of source tree
  2023-07-07 16:53       ` Stanislav Fomichev via Linux-kernel-mentees
@ 2023-07-09 14:45         ` Anh Tuan Phan
  -1 siblings, 0 replies; 24+ messages in thread
From: Anh Tuan Phan @ 2023-07-09 14:45 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: bpf, ast, daniel, andrii, martin.lau, linux-kernel-mentees

I updated the patch to reflect your suggestion. Thank you!

On 7/7/23 23:53, Stanislav Fomichev wrote:
> On 07/07, Anh Tuan Phan wrote:
>>
>>
>> On 7/7/23 01:09, Stanislav Fomichev wrote:
>>> On 07/06, Anh Tuan Phan wrote:
>>>> This commit fixes a few compilation issues when building out of source
>>>> tree. The command that I used to build samples/bpf:
>>>>
>>>> export KBUILD_OUTPUT=/tmp
>>>> make V=1 M=samples/bpf
>>>>
>>>> The compilation failed since it tried to find the header files in the
>>>> wrong places between output directory and source tree directory
>>>>
>>>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
>>>> ---
>>>>  samples/bpf/Makefile        | 8 ++++----
>>>>  samples/bpf/Makefile.target | 2 +-
>>>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>>>> index 615f24ebc49c..32469aaa82d5 100644
>>>> --- a/samples/bpf/Makefile
>>>> +++ b/samples/bpf/Makefile
>>>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>>>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
>>>>  # TPROGS_CFLAGS causes conflicts
>>>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
>>>> -		     -I$(src)/../../tools/include \
>>>> +		     -I$(srctree)/tools/include \
>>>
>>> [..]
>>>
>>>>  		     -I$(src)/../../tools/include/uapi \
>>>
>>> Does this $(src) need to be changed as well?
>>
>> I think this line doesn't affect the build. I removed it and it still
>> compiles (after "make -C samples/bpf clean"). I guess xdp_sample_user.c
>> doesn't include any file in tools/include/uapi. Am I missing something
>> or should I remove this line?
> 
> You might have these headers installed on your system already if
> it compiles without this part. So I'd keep this part but do
> s/src/srctree/ (and remove ../..).
> 
>>>
>>>
>>>>  		     -I$(LIBBPF_INCLUDE) \
>>>> -		     -I$(src)/../../tools/testing/selftests/bpf
>>>> +		     -I$(srctree)/tools/testing/selftests/bpf
>>>>
>>>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
>>>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
>>>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
>>>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
>>>> $(src)/xdp_sample_shared.h
>>>>  	@echo "  CLANG-BPF " $@
>>>>  	$(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
>>>> -		-Wno-compare-distinct-pointer-types -I$(srctree)/include \
>>>> +		-Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
>>>>  		-I$(srctree)/samples/bpf -I$(srctree)/tools/include \
>>>>  		-I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
>>>>  		-c $(filter %.bpf.c,$^) -o $@
>>>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
>>>> xdp_sample.bpf.o
>>>>
>>>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
>>>> skel,$(LINKED_SKELS),$($(skel)-deps)))
>>>>
>>>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
>>>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
>>>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
>>>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
>>>>
>>>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
>>>> index 7621f55e2947..86a454cfb080 100644
>>>> --- a/samples/bpf/Makefile.target
>>>> +++ b/samples/bpf/Makefile.target
>>>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
>>>>                   $(TPROGCFLAGS_$(basetarget).o)
>>>>
>>>>  # $(objtree)/$(obj) for including generated headers from checkin source
>>>> files
>>>
>>> [..]
>>>
>>>> -ifeq ($(KBUILD_EXTMOD),)
>>>> +ifneq ($(KBUILD_EXTMOD),)
>>>
>>> This parts seems to be copy-pasted all over the place in its 'ifeq'
>>> form. What is it doing and why is it needed?
>>>
>>>>  ifdef building_out_of_srctree
>>>>  _tprogc_flags   += -I $(objtree)/$(obj)
>>>>  endif
>>>> -- 
>>>> 2.34.1

Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
---

Change from the original patch

- Change "-I$(src)/../../tools/include/uapi" to
"-I$(srctree)/tools/include/uapi"

 samples/bpf/Makefile        | 10 +++++-----
 samples/bpf/Makefile.target |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 615f24ebc49c..cfc960b3713a 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
 # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
 # TPROGS_CFLAGS causes conflicts
 XDP_SAMPLE_CFLAGS += -Wall -O2 \
-		     -I$(src)/../../tools/include \
-		     -I$(src)/../../tools/include/uapi \
+		     -I$(srctree)/tools/include \
+		     -I$(srctree)/tools/include/uapi \
 		     -I$(LIBBPF_INCLUDE) \
-		     -I$(src)/../../tools/testing/selftests/bpf
+		     -I$(srctree)/tools/testing/selftests/bpf

 $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
 $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
@@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
 $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
$(src)/xdp_sample_shared.h
 	@echo "  CLANG-BPF " $@
 	$(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
-		-Wno-compare-distinct-pointer-types -I$(srctree)/include \
+		-Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
 		-I$(srctree)/samples/bpf -I$(srctree)/tools/include \
 		-I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
 		-c $(filter %.bpf.c,$^) -o $@
@@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
xdp_sample.bpf.o

 LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
skel,$(LINKED_SKELS),$($(skel)-deps)))

-BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
+BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
 BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
 BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))

diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
index 7621f55e2947..86a454cfb080 100644
--- a/samples/bpf/Makefile.target
+++ b/samples/bpf/Makefile.target
@@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
                  $(TPROGCFLAGS_$(basetarget).o)

 # $(objtree)/$(obj) for including generated headers from checkin source
files
-ifeq ($(KBUILD_EXTMOD),)
+ifneq ($(KBUILD_EXTMOD),)
 ifdef building_out_of_srctree
 _tprogc_flags   += -I $(objtree)/$(obj)
 endif
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* Re: [PATCH v1] samples/bpf: Fix build out of source tree
@ 2023-07-09 14:45         ` Anh Tuan Phan
  0 siblings, 0 replies; 24+ messages in thread
From: Anh Tuan Phan @ 2023-07-09 14:45 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: daniel, martin.lau, ast, andrii, bpf, linux-kernel-mentees

I updated the patch to reflect your suggestion. Thank you!

On 7/7/23 23:53, Stanislav Fomichev wrote:
> On 07/07, Anh Tuan Phan wrote:
>>
>>
>> On 7/7/23 01:09, Stanislav Fomichev wrote:
>>> On 07/06, Anh Tuan Phan wrote:
>>>> This commit fixes a few compilation issues when building out of source
>>>> tree. The command that I used to build samples/bpf:
>>>>
>>>> export KBUILD_OUTPUT=/tmp
>>>> make V=1 M=samples/bpf
>>>>
>>>> The compilation failed since it tried to find the header files in the
>>>> wrong places between output directory and source tree directory
>>>>
>>>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
>>>> ---
>>>>  samples/bpf/Makefile        | 8 ++++----
>>>>  samples/bpf/Makefile.target | 2 +-
>>>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>>>> index 615f24ebc49c..32469aaa82d5 100644
>>>> --- a/samples/bpf/Makefile
>>>> +++ b/samples/bpf/Makefile
>>>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>>>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
>>>>  # TPROGS_CFLAGS causes conflicts
>>>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
>>>> -		     -I$(src)/../../tools/include \
>>>> +		     -I$(srctree)/tools/include \
>>>
>>> [..]
>>>
>>>>  		     -I$(src)/../../tools/include/uapi \
>>>
>>> Does this $(src) need to be changed as well?
>>
>> I think this line doesn't affect the build. I removed it and it still
>> compiles (after "make -C samples/bpf clean"). I guess xdp_sample_user.c
>> doesn't include any file in tools/include/uapi. Am I missing something
>> or should I remove this line?
> 
> You might have these headers installed on your system already if
> it compiles without this part. So I'd keep this part but do
> s/src/srctree/ (and remove ../..).
> 
>>>
>>>
>>>>  		     -I$(LIBBPF_INCLUDE) \
>>>> -		     -I$(src)/../../tools/testing/selftests/bpf
>>>> +		     -I$(srctree)/tools/testing/selftests/bpf
>>>>
>>>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
>>>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
>>>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
>>>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
>>>> $(src)/xdp_sample_shared.h
>>>>  	@echo "  CLANG-BPF " $@
>>>>  	$(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
>>>> -		-Wno-compare-distinct-pointer-types -I$(srctree)/include \
>>>> +		-Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
>>>>  		-I$(srctree)/samples/bpf -I$(srctree)/tools/include \
>>>>  		-I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
>>>>  		-c $(filter %.bpf.c,$^) -o $@
>>>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
>>>> xdp_sample.bpf.o
>>>>
>>>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
>>>> skel,$(LINKED_SKELS),$($(skel)-deps)))
>>>>
>>>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
>>>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
>>>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
>>>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
>>>>
>>>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
>>>> index 7621f55e2947..86a454cfb080 100644
>>>> --- a/samples/bpf/Makefile.target
>>>> +++ b/samples/bpf/Makefile.target
>>>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
>>>>                   $(TPROGCFLAGS_$(basetarget).o)
>>>>
>>>>  # $(objtree)/$(obj) for including generated headers from checkin source
>>>> files
>>>
>>> [..]
>>>
>>>> -ifeq ($(KBUILD_EXTMOD),)
>>>> +ifneq ($(KBUILD_EXTMOD),)
>>>
>>> This parts seems to be copy-pasted all over the place in its 'ifeq'
>>> form. What is it doing and why is it needed?
>>>
>>>>  ifdef building_out_of_srctree
>>>>  _tprogc_flags   += -I $(objtree)/$(obj)
>>>>  endif
>>>> -- 
>>>> 2.34.1

Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
---

Change from the original patch

- Change "-I$(src)/../../tools/include/uapi" to
"-I$(srctree)/tools/include/uapi"

 samples/bpf/Makefile        | 10 +++++-----
 samples/bpf/Makefile.target |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 615f24ebc49c..cfc960b3713a 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
 # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
 # TPROGS_CFLAGS causes conflicts
 XDP_SAMPLE_CFLAGS += -Wall -O2 \
-		     -I$(src)/../../tools/include \
-		     -I$(src)/../../tools/include/uapi \
+		     -I$(srctree)/tools/include \
+		     -I$(srctree)/tools/include/uapi \
 		     -I$(LIBBPF_INCLUDE) \
-		     -I$(src)/../../tools/testing/selftests/bpf
+		     -I$(srctree)/tools/testing/selftests/bpf

 $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
 $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
@@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
 $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
$(src)/xdp_sample_shared.h
 	@echo "  CLANG-BPF " $@
 	$(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
-		-Wno-compare-distinct-pointer-types -I$(srctree)/include \
+		-Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
 		-I$(srctree)/samples/bpf -I$(srctree)/tools/include \
 		-I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
 		-c $(filter %.bpf.c,$^) -o $@
@@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
xdp_sample.bpf.o

 LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
skel,$(LINKED_SKELS),$($(skel)-deps)))

-BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
+BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
 BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
 BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))

diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
index 7621f55e2947..86a454cfb080 100644
--- a/samples/bpf/Makefile.target
+++ b/samples/bpf/Makefile.target
@@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
                  $(TPROGCFLAGS_$(basetarget).o)

 # $(objtree)/$(obj) for including generated headers from checkin source
files
-ifeq ($(KBUILD_EXTMOD),)
+ifneq ($(KBUILD_EXTMOD),)
 ifdef building_out_of_srctree
 _tprogc_flags   += -I $(objtree)/$(obj)
 endif
-- 
2.34.1

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* Re: [PATCH v1] samples/bpf: Fix build out of source tree
  2023-07-09 14:45         ` Anh Tuan Phan
@ 2023-07-10 17:18           ` Stanislav Fomichev via Linux-kernel-mentees
  -1 siblings, 0 replies; 24+ messages in thread
From: Stanislav Fomichev @ 2023-07-10 17:18 UTC (permalink / raw)
  To: Anh Tuan Phan; +Cc: bpf, ast, daniel, andrii, martin.lau, linux-kernel-mentees

On 07/09, Anh Tuan Phan wrote:
> I updated the patch to reflect your suggestion. Thank you!

In the future, can you please post a new one with v+1 instead of replying
to the old one? Thx!

> On 7/7/23 23:53, Stanislav Fomichev wrote:
> > On 07/07, Anh Tuan Phan wrote:
> >>
> >>
> >> On 7/7/23 01:09, Stanislav Fomichev wrote:
> >>> On 07/06, Anh Tuan Phan wrote:
> >>>> This commit fixes a few compilation issues when building out of source
> >>>> tree. The command that I used to build samples/bpf:
> >>>>
> >>>> export KBUILD_OUTPUT=/tmp
> >>>> make V=1 M=samples/bpf
> >>>>
> >>>> The compilation failed since it tried to find the header files in the
> >>>> wrong places between output directory and source tree directory
> >>>>
> >>>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
> >>>> ---
> >>>>  samples/bpf/Makefile        | 8 ++++----
> >>>>  samples/bpf/Makefile.target | 2 +-
> >>>>  2 files changed, 5 insertions(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> >>>> index 615f24ebc49c..32469aaa82d5 100644
> >>>> --- a/samples/bpf/Makefile
> >>>> +++ b/samples/bpf/Makefile
> >>>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
> >>>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
> >>>>  # TPROGS_CFLAGS causes conflicts
> >>>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
> >>>> -		     -I$(src)/../../tools/include \
> >>>> +		     -I$(srctree)/tools/include \
> >>>
> >>> [..]
> >>>
> >>>>  		     -I$(src)/../../tools/include/uapi \
> >>>
> >>> Does this $(src) need to be changed as well?
> >>
> >> I think this line doesn't affect the build. I removed it and it still
> >> compiles (after "make -C samples/bpf clean"). I guess xdp_sample_user.c
> >> doesn't include any file in tools/include/uapi. Am I missing something
> >> or should I remove this line?
> > 
> > You might have these headers installed on your system already if
> > it compiles without this part. So I'd keep this part but do
> > s/src/srctree/ (and remove ../..).
> > 
> >>>
> >>>
> >>>>  		     -I$(LIBBPF_INCLUDE) \
> >>>> -		     -I$(src)/../../tools/testing/selftests/bpf
> >>>> +		     -I$(srctree)/tools/testing/selftests/bpf
> >>>>
> >>>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
> >>>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
> >>>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
> >>>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
> >>>> $(src)/xdp_sample_shared.h
> >>>>  	@echo "  CLANG-BPF " $@
> >>>>  	$(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
> >>>> -		-Wno-compare-distinct-pointer-types -I$(srctree)/include \
> >>>> +		-Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
> >>>>  		-I$(srctree)/samples/bpf -I$(srctree)/tools/include \
> >>>>  		-I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
> >>>>  		-c $(filter %.bpf.c,$^) -o $@
> >>>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
> >>>> xdp_sample.bpf.o
> >>>>
> >>>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
> >>>> skel,$(LINKED_SKELS),$($(skel)-deps)))
> >>>>
> >>>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
> >>>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
> >>>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
> >>>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
> >>>>
> >>>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
> >>>> index 7621f55e2947..86a454cfb080 100644
> >>>> --- a/samples/bpf/Makefile.target
> >>>> +++ b/samples/bpf/Makefile.target
> >>>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
> >>>>                   $(TPROGCFLAGS_$(basetarget).o)
> >>>>
> >>>>  # $(objtree)/$(obj) for including generated headers from checkin source
> >>>> files
> >>>
> >>> [..]
> >>>
> >>>> -ifeq ($(KBUILD_EXTMOD),)
> >>>> +ifneq ($(KBUILD_EXTMOD),)
> >>>
> >>> This parts seems to be copy-pasted all over the place in its 'ifeq'
> >>> form. What is it doing and why is it needed?
> >>>
> >>>>  ifdef building_out_of_srctree
> >>>>  _tprogc_flags   += -I $(objtree)/$(obj)
> >>>>  endif
> >>>> -- 
> >>>> 2.34.1
> 
> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
> ---
> 
> Change from the original patch
> 
> - Change "-I$(src)/../../tools/include/uapi" to
> "-I$(srctree)/tools/include/uapi"
> 
>  samples/bpf/Makefile        | 10 +++++-----
>  samples/bpf/Makefile.target |  2 +-
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 615f24ebc49c..cfc960b3713a 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
>  # TPROGS_CFLAGS causes conflicts
>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
> -		     -I$(src)/../../tools/include \
> -		     -I$(src)/../../tools/include/uapi \
> +		     -I$(srctree)/tools/include \
> +		     -I$(srctree)/tools/include/uapi \
>  		     -I$(LIBBPF_INCLUDE) \
> -		     -I$(src)/../../tools/testing/selftests/bpf
> +		     -I$(srctree)/tools/testing/selftests/bpf
> 
>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
> $(src)/xdp_sample_shared.h
>  	@echo "  CLANG-BPF " $@
>  	$(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
> -		-Wno-compare-distinct-pointer-types -I$(srctree)/include \
> +		-Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
>  		-I$(srctree)/samples/bpf -I$(srctree)/tools/include \
>  		-I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
>  		-c $(filter %.bpf.c,$^) -o $@
> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
> xdp_sample.bpf.o
> 
>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
> skel,$(LINKED_SKELS),$($(skel)-deps)))
> 
> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
> 
> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
> index 7621f55e2947..86a454cfb080 100644
> --- a/samples/bpf/Makefile.target
> +++ b/samples/bpf/Makefile.target
> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
>                   $(TPROGCFLAGS_$(basetarget).o)
> 
>  # $(objtree)/$(obj) for including generated headers from checkin source
> files

[..]

> -ifeq ($(KBUILD_EXTMOD),)
> +ifneq ($(KBUILD_EXTMOD),)
>  ifdef building_out_of_srctree
>  _tprogc_flags   += -I $(objtree)/$(obj)
>  endif

This question left undressed. Can you share more on why this change
is needed? Because it looks like it's actually needed for M='' case.
IOW, maybe we should add $(objtree)/$(obj) somewhere else?

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v1] samples/bpf: Fix build out of source tree
@ 2023-07-10 17:18           ` Stanislav Fomichev via Linux-kernel-mentees
  0 siblings, 0 replies; 24+ messages in thread
From: Stanislav Fomichev via Linux-kernel-mentees @ 2023-07-10 17:18 UTC (permalink / raw)
  To: Anh Tuan Phan; +Cc: daniel, martin.lau, ast, andrii, bpf, linux-kernel-mentees

On 07/09, Anh Tuan Phan wrote:
> I updated the patch to reflect your suggestion. Thank you!

In the future, can you please post a new one with v+1 instead of replying
to the old one? Thx!

> On 7/7/23 23:53, Stanislav Fomichev wrote:
> > On 07/07, Anh Tuan Phan wrote:
> >>
> >>
> >> On 7/7/23 01:09, Stanislav Fomichev wrote:
> >>> On 07/06, Anh Tuan Phan wrote:
> >>>> This commit fixes a few compilation issues when building out of source
> >>>> tree. The command that I used to build samples/bpf:
> >>>>
> >>>> export KBUILD_OUTPUT=/tmp
> >>>> make V=1 M=samples/bpf
> >>>>
> >>>> The compilation failed since it tried to find the header files in the
> >>>> wrong places between output directory and source tree directory
> >>>>
> >>>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
> >>>> ---
> >>>>  samples/bpf/Makefile        | 8 ++++----
> >>>>  samples/bpf/Makefile.target | 2 +-
> >>>>  2 files changed, 5 insertions(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> >>>> index 615f24ebc49c..32469aaa82d5 100644
> >>>> --- a/samples/bpf/Makefile
> >>>> +++ b/samples/bpf/Makefile
> >>>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
> >>>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
> >>>>  # TPROGS_CFLAGS causes conflicts
> >>>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
> >>>> -		     -I$(src)/../../tools/include \
> >>>> +		     -I$(srctree)/tools/include \
> >>>
> >>> [..]
> >>>
> >>>>  		     -I$(src)/../../tools/include/uapi \
> >>>
> >>> Does this $(src) need to be changed as well?
> >>
> >> I think this line doesn't affect the build. I removed it and it still
> >> compiles (after "make -C samples/bpf clean"). I guess xdp_sample_user.c
> >> doesn't include any file in tools/include/uapi. Am I missing something
> >> or should I remove this line?
> > 
> > You might have these headers installed on your system already if
> > it compiles without this part. So I'd keep this part but do
> > s/src/srctree/ (and remove ../..).
> > 
> >>>
> >>>
> >>>>  		     -I$(LIBBPF_INCLUDE) \
> >>>> -		     -I$(src)/../../tools/testing/selftests/bpf
> >>>> +		     -I$(srctree)/tools/testing/selftests/bpf
> >>>>
> >>>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
> >>>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
> >>>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
> >>>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
> >>>> $(src)/xdp_sample_shared.h
> >>>>  	@echo "  CLANG-BPF " $@
> >>>>  	$(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
> >>>> -		-Wno-compare-distinct-pointer-types -I$(srctree)/include \
> >>>> +		-Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
> >>>>  		-I$(srctree)/samples/bpf -I$(srctree)/tools/include \
> >>>>  		-I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
> >>>>  		-c $(filter %.bpf.c,$^) -o $@
> >>>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
> >>>> xdp_sample.bpf.o
> >>>>
> >>>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
> >>>> skel,$(LINKED_SKELS),$($(skel)-deps)))
> >>>>
> >>>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
> >>>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
> >>>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
> >>>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
> >>>>
> >>>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
> >>>> index 7621f55e2947..86a454cfb080 100644
> >>>> --- a/samples/bpf/Makefile.target
> >>>> +++ b/samples/bpf/Makefile.target
> >>>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
> >>>>                   $(TPROGCFLAGS_$(basetarget).o)
> >>>>
> >>>>  # $(objtree)/$(obj) for including generated headers from checkin source
> >>>> files
> >>>
> >>> [..]
> >>>
> >>>> -ifeq ($(KBUILD_EXTMOD),)
> >>>> +ifneq ($(KBUILD_EXTMOD),)
> >>>
> >>> This parts seems to be copy-pasted all over the place in its 'ifeq'
> >>> form. What is it doing and why is it needed?
> >>>
> >>>>  ifdef building_out_of_srctree
> >>>>  _tprogc_flags   += -I $(objtree)/$(obj)
> >>>>  endif
> >>>> -- 
> >>>> 2.34.1
> 
> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
> ---
> 
> Change from the original patch
> 
> - Change "-I$(src)/../../tools/include/uapi" to
> "-I$(srctree)/tools/include/uapi"
> 
>  samples/bpf/Makefile        | 10 +++++-----
>  samples/bpf/Makefile.target |  2 +-
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 615f24ebc49c..cfc960b3713a 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
>  # TPROGS_CFLAGS causes conflicts
>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
> -		     -I$(src)/../../tools/include \
> -		     -I$(src)/../../tools/include/uapi \
> +		     -I$(srctree)/tools/include \
> +		     -I$(srctree)/tools/include/uapi \
>  		     -I$(LIBBPF_INCLUDE) \
> -		     -I$(src)/../../tools/testing/selftests/bpf
> +		     -I$(srctree)/tools/testing/selftests/bpf
> 
>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
> $(src)/xdp_sample_shared.h
>  	@echo "  CLANG-BPF " $@
>  	$(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
> -		-Wno-compare-distinct-pointer-types -I$(srctree)/include \
> +		-Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
>  		-I$(srctree)/samples/bpf -I$(srctree)/tools/include \
>  		-I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
>  		-c $(filter %.bpf.c,$^) -o $@
> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
> xdp_sample.bpf.o
> 
>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
> skel,$(LINKED_SKELS),$($(skel)-deps)))
> 
> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
> 
> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
> index 7621f55e2947..86a454cfb080 100644
> --- a/samples/bpf/Makefile.target
> +++ b/samples/bpf/Makefile.target
> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
>                   $(TPROGCFLAGS_$(basetarget).o)
> 
>  # $(objtree)/$(obj) for including generated headers from checkin source
> files

[..]

> -ifeq ($(KBUILD_EXTMOD),)
> +ifneq ($(KBUILD_EXTMOD),)
>  ifdef building_out_of_srctree
>  _tprogc_flags   += -I $(objtree)/$(obj)
>  endif

This question left undressed. Can you share more on why this change
is needed? Because it looks like it's actually needed for M='' case.
IOW, maybe we should add $(objtree)/$(obj) somewhere else?
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v1] samples/bpf: Fix build out of source tree
  2023-07-10 17:18           ` Stanislav Fomichev via Linux-kernel-mentees
@ 2023-07-16  9:42             ` Anh Tuan Phan
  -1 siblings, 0 replies; 24+ messages in thread
From: Anh Tuan Phan @ 2023-07-16  9:42 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: bpf, ast, daniel, andrii, martin.lau, linux-kernel-mentees



On 7/11/23 00:18, Stanislav Fomichev wrote:
> On 07/09, Anh Tuan Phan wrote:
>> I updated the patch to reflect your suggestion. Thank you!
> 
> In the future, can you please post a new one with v+1 instead of replying
> to the old one? Thx!

Will do in the next version.

> 
>> On 7/7/23 23:53, Stanislav Fomichev wrote:
>>> On 07/07, Anh Tuan Phan wrote:
>>>>
>>>>
>>>> On 7/7/23 01:09, Stanislav Fomichev wrote:
>>>>> On 07/06, Anh Tuan Phan wrote:
>>>>>> This commit fixes a few compilation issues when building out of source
>>>>>> tree. The command that I used to build samples/bpf:
>>>>>>
>>>>>> export KBUILD_OUTPUT=/tmp
>>>>>> make V=1 M=samples/bpf
>>>>>>
>>>>>> The compilation failed since it tried to find the header files in the
>>>>>> wrong places between output directory and source tree directory
>>>>>>
>>>>>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
>>>>>> ---
>>>>>>  samples/bpf/Makefile        | 8 ++++----
>>>>>>  samples/bpf/Makefile.target | 2 +-
>>>>>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>>>>>> index 615f24ebc49c..32469aaa82d5 100644
>>>>>> --- a/samples/bpf/Makefile
>>>>>> +++ b/samples/bpf/Makefile
>>>>>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>>>>>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
>>>>>>  # TPROGS_CFLAGS causes conflicts
>>>>>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
>>>>>> -		     -I$(src)/../../tools/include \
>>>>>> +		     -I$(srctree)/tools/include \
>>>>>
>>>>> [..]
>>>>>
>>>>>>  		     -I$(src)/../../tools/include/uapi \
>>>>>
>>>>> Does this $(src) need to be changed as well?
>>>>
>>>> I think this line doesn't affect the build. I removed it and it still
>>>> compiles (after "make -C samples/bpf clean"). I guess xdp_sample_user.c
>>>> doesn't include any file in tools/include/uapi. Am I missing something
>>>> or should I remove this line?
>>>
>>> You might have these headers installed on your system already if
>>> it compiles without this part. So I'd keep this part but do
>>> s/src/srctree/ (and remove ../..).
>>>
>>>>>
>>>>>
>>>>>>  		     -I$(LIBBPF_INCLUDE) \
>>>>>> -		     -I$(src)/../../tools/testing/selftests/bpf
>>>>>> +		     -I$(srctree)/tools/testing/selftests/bpf
>>>>>>
>>>>>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
>>>>>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
>>>>>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
>>>>>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
>>>>>> $(src)/xdp_sample_shared.h
>>>>>>  	@echo "  CLANG-BPF " $@
>>>>>>  	$(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
>>>>>> -		-Wno-compare-distinct-pointer-types -I$(srctree)/include \
>>>>>> +		-Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
>>>>>>  		-I$(srctree)/samples/bpf -I$(srctree)/tools/include \
>>>>>>  		-I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
>>>>>>  		-c $(filter %.bpf.c,$^) -o $@
>>>>>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
>>>>>> xdp_sample.bpf.o
>>>>>>
>>>>>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
>>>>>> skel,$(LINKED_SKELS),$($(skel)-deps)))
>>>>>>
>>>>>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
>>>>>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
>>>>>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
>>>>>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
>>>>>>
>>>>>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
>>>>>> index 7621f55e2947..86a454cfb080 100644
>>>>>> --- a/samples/bpf/Makefile.target
>>>>>> +++ b/samples/bpf/Makefile.target
>>>>>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
>>>>>>                   $(TPROGCFLAGS_$(basetarget).o)
>>>>>>
>>>>>>  # $(objtree)/$(obj) for including generated headers from checkin source
>>>>>> files
>>>>>
>>>>> [..]
>>>>>
>>>>>> -ifeq ($(KBUILD_EXTMOD),)
>>>>>> +ifneq ($(KBUILD_EXTMOD),)
>>>>>
>>>>> This parts seems to be copy-pasted all over the place in its 'ifeq'
>>>>> form. What is it doing and why is it needed?
>>>>>
>>>>>>  ifdef building_out_of_srctree
>>>>>>  _tprogc_flags   += -I $(objtree)/$(obj)
>>>>>>  endif
>>>>>> -- 
>>>>>> 2.34.1
>>
>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
>> ---
>>
>> Change from the original patch
>>
>> - Change "-I$(src)/../../tools/include/uapi" to
>> "-I$(srctree)/tools/include/uapi"
>>
>>  samples/bpf/Makefile        | 10 +++++-----
>>  samples/bpf/Makefile.target |  2 +-
>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> index 615f24ebc49c..cfc960b3713a 100644
>> --- a/samples/bpf/Makefile
>> +++ b/samples/bpf/Makefile
>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
>>  # TPROGS_CFLAGS causes conflicts
>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
>> -		     -I$(src)/../../tools/include \
>> -		     -I$(src)/../../tools/include/uapi \
>> +		     -I$(srctree)/tools/include \
>> +		     -I$(srctree)/tools/include/uapi \
>>  		     -I$(LIBBPF_INCLUDE) \
>> -		     -I$(src)/../../tools/testing/selftests/bpf
>> +		     -I$(srctree)/tools/testing/selftests/bpf
>>
>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
>> $(src)/xdp_sample_shared.h
>>  	@echo "  CLANG-BPF " $@
>>  	$(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
>> -		-Wno-compare-distinct-pointer-types -I$(srctree)/include \
>> +		-Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
>>  		-I$(srctree)/samples/bpf -I$(srctree)/tools/include \
>>  		-I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
>>  		-c $(filter %.bpf.c,$^) -o $@
>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
>> xdp_sample.bpf.o
>>
>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
>> skel,$(LINKED_SKELS),$($(skel)-deps)))
>>
>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
>>
>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
>> index 7621f55e2947..86a454cfb080 100644
>> --- a/samples/bpf/Makefile.target
>> +++ b/samples/bpf/Makefile.target
>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
>>                   $(TPROGCFLAGS_$(basetarget).o)
>>
>>  # $(objtree)/$(obj) for including generated headers from checkin source
>> files
> 
> [..]
> 
>> -ifeq ($(KBUILD_EXTMOD),)
>> +ifneq ($(KBUILD_EXTMOD),)
>>  ifdef building_out_of_srctree
>>  _tprogc_flags   += -I $(objtree)/$(obj)
>>  endif
> 
> This question left undressed. Can you share more on why this change
> is needed? Because it looks like it's actually needed for M='' case.
> IOW, maybe we should add $(objtree)/$(obj) somewhere else?

If it's needed for both cases M='' and M != '', is it better to just
remove the condition with $(KBUILD_EXTMOD)? FMPOV, -I $(objtree)/$(obj)
is only needed in case of building_out_of_srctree, no matter what
KBUILD_EXTMOD.

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v1] samples/bpf: Fix build out of source tree
@ 2023-07-16  9:42             ` Anh Tuan Phan
  0 siblings, 0 replies; 24+ messages in thread
From: Anh Tuan Phan @ 2023-07-16  9:42 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: daniel, martin.lau, ast, andrii, bpf, linux-kernel-mentees



On 7/11/23 00:18, Stanislav Fomichev wrote:
> On 07/09, Anh Tuan Phan wrote:
>> I updated the patch to reflect your suggestion. Thank you!
> 
> In the future, can you please post a new one with v+1 instead of replying
> to the old one? Thx!

Will do in the next version.

> 
>> On 7/7/23 23:53, Stanislav Fomichev wrote:
>>> On 07/07, Anh Tuan Phan wrote:
>>>>
>>>>
>>>> On 7/7/23 01:09, Stanislav Fomichev wrote:
>>>>> On 07/06, Anh Tuan Phan wrote:
>>>>>> This commit fixes a few compilation issues when building out of source
>>>>>> tree. The command that I used to build samples/bpf:
>>>>>>
>>>>>> export KBUILD_OUTPUT=/tmp
>>>>>> make V=1 M=samples/bpf
>>>>>>
>>>>>> The compilation failed since it tried to find the header files in the
>>>>>> wrong places between output directory and source tree directory
>>>>>>
>>>>>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
>>>>>> ---
>>>>>>  samples/bpf/Makefile        | 8 ++++----
>>>>>>  samples/bpf/Makefile.target | 2 +-
>>>>>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>>>>>> index 615f24ebc49c..32469aaa82d5 100644
>>>>>> --- a/samples/bpf/Makefile
>>>>>> +++ b/samples/bpf/Makefile
>>>>>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>>>>>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
>>>>>>  # TPROGS_CFLAGS causes conflicts
>>>>>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
>>>>>> -		     -I$(src)/../../tools/include \
>>>>>> +		     -I$(srctree)/tools/include \
>>>>>
>>>>> [..]
>>>>>
>>>>>>  		     -I$(src)/../../tools/include/uapi \
>>>>>
>>>>> Does this $(src) need to be changed as well?
>>>>
>>>> I think this line doesn't affect the build. I removed it and it still
>>>> compiles (after "make -C samples/bpf clean"). I guess xdp_sample_user.c
>>>> doesn't include any file in tools/include/uapi. Am I missing something
>>>> or should I remove this line?
>>>
>>> You might have these headers installed on your system already if
>>> it compiles without this part. So I'd keep this part but do
>>> s/src/srctree/ (and remove ../..).
>>>
>>>>>
>>>>>
>>>>>>  		     -I$(LIBBPF_INCLUDE) \
>>>>>> -		     -I$(src)/../../tools/testing/selftests/bpf
>>>>>> +		     -I$(srctree)/tools/testing/selftests/bpf
>>>>>>
>>>>>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
>>>>>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
>>>>>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
>>>>>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
>>>>>> $(src)/xdp_sample_shared.h
>>>>>>  	@echo "  CLANG-BPF " $@
>>>>>>  	$(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
>>>>>> -		-Wno-compare-distinct-pointer-types -I$(srctree)/include \
>>>>>> +		-Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
>>>>>>  		-I$(srctree)/samples/bpf -I$(srctree)/tools/include \
>>>>>>  		-I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
>>>>>>  		-c $(filter %.bpf.c,$^) -o $@
>>>>>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
>>>>>> xdp_sample.bpf.o
>>>>>>
>>>>>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
>>>>>> skel,$(LINKED_SKELS),$($(skel)-deps)))
>>>>>>
>>>>>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
>>>>>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
>>>>>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
>>>>>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
>>>>>>
>>>>>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
>>>>>> index 7621f55e2947..86a454cfb080 100644
>>>>>> --- a/samples/bpf/Makefile.target
>>>>>> +++ b/samples/bpf/Makefile.target
>>>>>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
>>>>>>                   $(TPROGCFLAGS_$(basetarget).o)
>>>>>>
>>>>>>  # $(objtree)/$(obj) for including generated headers from checkin source
>>>>>> files
>>>>>
>>>>> [..]
>>>>>
>>>>>> -ifeq ($(KBUILD_EXTMOD),)
>>>>>> +ifneq ($(KBUILD_EXTMOD),)
>>>>>
>>>>> This parts seems to be copy-pasted all over the place in its 'ifeq'
>>>>> form. What is it doing and why is it needed?
>>>>>
>>>>>>  ifdef building_out_of_srctree
>>>>>>  _tprogc_flags   += -I $(objtree)/$(obj)
>>>>>>  endif
>>>>>> -- 
>>>>>> 2.34.1
>>
>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
>> ---
>>
>> Change from the original patch
>>
>> - Change "-I$(src)/../../tools/include/uapi" to
>> "-I$(srctree)/tools/include/uapi"
>>
>>  samples/bpf/Makefile        | 10 +++++-----
>>  samples/bpf/Makefile.target |  2 +-
>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> index 615f24ebc49c..cfc960b3713a 100644
>> --- a/samples/bpf/Makefile
>> +++ b/samples/bpf/Makefile
>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
>>  # TPROGS_CFLAGS causes conflicts
>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
>> -		     -I$(src)/../../tools/include \
>> -		     -I$(src)/../../tools/include/uapi \
>> +		     -I$(srctree)/tools/include \
>> +		     -I$(srctree)/tools/include/uapi \
>>  		     -I$(LIBBPF_INCLUDE) \
>> -		     -I$(src)/../../tools/testing/selftests/bpf
>> +		     -I$(srctree)/tools/testing/selftests/bpf
>>
>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
>> $(src)/xdp_sample_shared.h
>>  	@echo "  CLANG-BPF " $@
>>  	$(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
>> -		-Wno-compare-distinct-pointer-types -I$(srctree)/include \
>> +		-Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
>>  		-I$(srctree)/samples/bpf -I$(srctree)/tools/include \
>>  		-I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
>>  		-c $(filter %.bpf.c,$^) -o $@
>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
>> xdp_sample.bpf.o
>>
>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
>> skel,$(LINKED_SKELS),$($(skel)-deps)))
>>
>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
>>
>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
>> index 7621f55e2947..86a454cfb080 100644
>> --- a/samples/bpf/Makefile.target
>> +++ b/samples/bpf/Makefile.target
>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
>>                   $(TPROGCFLAGS_$(basetarget).o)
>>
>>  # $(objtree)/$(obj) for including generated headers from checkin source
>> files
> 
> [..]
> 
>> -ifeq ($(KBUILD_EXTMOD),)
>> +ifneq ($(KBUILD_EXTMOD),)
>>  ifdef building_out_of_srctree
>>  _tprogc_flags   += -I $(objtree)/$(obj)
>>  endif
> 
> This question left undressed. Can you share more on why this change
> is needed? Because it looks like it's actually needed for M='' case.
> IOW, maybe we should add $(objtree)/$(obj) somewhere else?

If it's needed for both cases M='' and M != '', is it better to just
remove the condition with $(KBUILD_EXTMOD)? FMPOV, -I $(objtree)/$(obj)
is only needed in case of building_out_of_srctree, no matter what
KBUILD_EXTMOD.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v1] samples/bpf: Fix build out of source tree
  2023-07-16  9:42             ` Anh Tuan Phan
@ 2023-07-17 16:46               ` Stanislav Fomichev via Linux-kernel-mentees
  -1 siblings, 0 replies; 24+ messages in thread
From: Stanislav Fomichev @ 2023-07-17 16:46 UTC (permalink / raw)
  To: Anh Tuan Phan; +Cc: bpf, ast, daniel, andrii, martin.lau, linux-kernel-mentees

On Sun, Jul 16, 2023 at 2:42 AM Anh Tuan Phan <tuananhlfc@gmail.com> wrote:
>
>
>
> On 7/11/23 00:18, Stanislav Fomichev wrote:
> > On 07/09, Anh Tuan Phan wrote:
> >> I updated the patch to reflect your suggestion. Thank you!
> >
> > In the future, can you please post a new one with v+1 instead of replying
> > to the old one? Thx!
>
> Will do in the next version.
>
> >
> >> On 7/7/23 23:53, Stanislav Fomichev wrote:
> >>> On 07/07, Anh Tuan Phan wrote:
> >>>>
> >>>>
> >>>> On 7/7/23 01:09, Stanislav Fomichev wrote:
> >>>>> On 07/06, Anh Tuan Phan wrote:
> >>>>>> This commit fixes a few compilation issues when building out of source
> >>>>>> tree. The command that I used to build samples/bpf:
> >>>>>>
> >>>>>> export KBUILD_OUTPUT=/tmp
> >>>>>> make V=1 M=samples/bpf
> >>>>>>
> >>>>>> The compilation failed since it tried to find the header files in the
> >>>>>> wrong places between output directory and source tree directory
> >>>>>>
> >>>>>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
> >>>>>> ---
> >>>>>>  samples/bpf/Makefile        | 8 ++++----
> >>>>>>  samples/bpf/Makefile.target | 2 +-
> >>>>>>  2 files changed, 5 insertions(+), 5 deletions(-)
> >>>>>>
> >>>>>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> >>>>>> index 615f24ebc49c..32469aaa82d5 100644
> >>>>>> --- a/samples/bpf/Makefile
> >>>>>> +++ b/samples/bpf/Makefile
> >>>>>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
> >>>>>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
> >>>>>>  # TPROGS_CFLAGS causes conflicts
> >>>>>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
> >>>>>> -                     -I$(src)/../../tools/include \
> >>>>>> +                     -I$(srctree)/tools/include \
> >>>>>
> >>>>> [..]
> >>>>>
> >>>>>>                       -I$(src)/../../tools/include/uapi \
> >>>>>
> >>>>> Does this $(src) need to be changed as well?
> >>>>
> >>>> I think this line doesn't affect the build. I removed it and it still
> >>>> compiles (after "make -C samples/bpf clean"). I guess xdp_sample_user.c
> >>>> doesn't include any file in tools/include/uapi. Am I missing something
> >>>> or should I remove this line?
> >>>
> >>> You might have these headers installed on your system already if
> >>> it compiles without this part. So I'd keep this part but do
> >>> s/src/srctree/ (and remove ../..).
> >>>
> >>>>>
> >>>>>
> >>>>>>                       -I$(LIBBPF_INCLUDE) \
> >>>>>> -                     -I$(src)/../../tools/testing/selftests/bpf
> >>>>>> +                     -I$(srctree)/tools/testing/selftests/bpf
> >>>>>>
> >>>>>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
> >>>>>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
> >>>>>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
> >>>>>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
> >>>>>> $(src)/xdp_sample_shared.h
> >>>>>>          @echo "  CLANG-BPF " $@
> >>>>>>          $(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
> >>>>>> -                -Wno-compare-distinct-pointer-types -I$(srctree)/include \
> >>>>>> +                -Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
> >>>>>>                  -I$(srctree)/samples/bpf -I$(srctree)/tools/include \
> >>>>>>                  -I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
> >>>>>>                  -c $(filter %.bpf.c,$^) -o $@
> >>>>>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
> >>>>>> xdp_sample.bpf.o
> >>>>>>
> >>>>>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
> >>>>>> skel,$(LINKED_SKELS),$($(skel)-deps)))
> >>>>>>
> >>>>>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
> >>>>>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
> >>>>>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
> >>>>>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
> >>>>>>
> >>>>>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
> >>>>>> index 7621f55e2947..86a454cfb080 100644
> >>>>>> --- a/samples/bpf/Makefile.target
> >>>>>> +++ b/samples/bpf/Makefile.target
> >>>>>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
> >>>>>>                   $(TPROGCFLAGS_$(basetarget).o)
> >>>>>>
> >>>>>>  # $(objtree)/$(obj) for including generated headers from checkin source
> >>>>>> files
> >>>>>
> >>>>> [..]
> >>>>>
> >>>>>> -ifeq ($(KBUILD_EXTMOD),)
> >>>>>> +ifneq ($(KBUILD_EXTMOD),)
> >>>>>
> >>>>> This parts seems to be copy-pasted all over the place in its 'ifeq'
> >>>>> form. What is it doing and why is it needed?
> >>>>>
> >>>>>>  ifdef building_out_of_srctree
> >>>>>>  _tprogc_flags   += -I $(objtree)/$(obj)
> >>>>>>  endif
> >>>>>> --
> >>>>>> 2.34.1
> >>
> >> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
> >> ---
> >>
> >> Change from the original patch
> >>
> >> - Change "-I$(src)/../../tools/include/uapi" to
> >> "-I$(srctree)/tools/include/uapi"
> >>
> >>  samples/bpf/Makefile        | 10 +++++-----
> >>  samples/bpf/Makefile.target |  2 +-
> >>  2 files changed, 6 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> >> index 615f24ebc49c..cfc960b3713a 100644
> >> --- a/samples/bpf/Makefile
> >> +++ b/samples/bpf/Makefile
> >> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
> >>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
> >>  # TPROGS_CFLAGS causes conflicts
> >>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
> >> -                 -I$(src)/../../tools/include \
> >> -                 -I$(src)/../../tools/include/uapi \
> >> +                 -I$(srctree)/tools/include \
> >> +                 -I$(srctree)/tools/include/uapi \
> >>                   -I$(LIBBPF_INCLUDE) \
> >> -                 -I$(src)/../../tools/testing/selftests/bpf
> >> +                 -I$(srctree)/tools/testing/selftests/bpf
> >>
> >>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
> >>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
> >> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
> >>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
> >> $(src)/xdp_sample_shared.h
> >>      @echo "  CLANG-BPF " $@
> >>      $(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
> >> -            -Wno-compare-distinct-pointer-types -I$(srctree)/include \
> >> +            -Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
> >>              -I$(srctree)/samples/bpf -I$(srctree)/tools/include \
> >>              -I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
> >>              -c $(filter %.bpf.c,$^) -o $@
> >> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
> >> xdp_sample.bpf.o
> >>
> >>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
> >> skel,$(LINKED_SKELS),$($(skel)-deps)))
> >>
> >> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
> >> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
> >>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
> >>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
> >>
> >> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
> >> index 7621f55e2947..86a454cfb080 100644
> >> --- a/samples/bpf/Makefile.target
> >> +++ b/samples/bpf/Makefile.target
> >> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
> >>                   $(TPROGCFLAGS_$(basetarget).o)
> >>
> >>  # $(objtree)/$(obj) for including generated headers from checkin source
> >> files
> >
> > [..]
> >
> >> -ifeq ($(KBUILD_EXTMOD),)
> >> +ifneq ($(KBUILD_EXTMOD),)
> >>  ifdef building_out_of_srctree
> >>  _tprogc_flags   += -I $(objtree)/$(obj)
> >>  endif
> >
> > This question left undressed. Can you share more on why this change
> > is needed? Because it looks like it's actually needed for M='' case.
> > IOW, maybe we should add $(objtree)/$(obj) somewhere else?
>
> If it's needed for both cases M='' and M != '', is it better to just
> remove the condition with $(KBUILD_EXTMOD)? FMPOV, -I $(objtree)/$(obj)
> is only needed in case of building_out_of_srctree, no matter what
> KBUILD_EXTMOD.

My guess is that we're missing -I $(objtree)/$(obj) somewhere else.
Depending on what/where it fails, we probably need to add $(objtree)
to some of the $(obj)s?
I'm mostly speculating here, because I see that "ifeq
($(KBUILD_EXTMOD),) + ifdef building_out_of_srctree" pattern being
used in the other places.

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v1] samples/bpf: Fix build out of source tree
@ 2023-07-17 16:46               ` Stanislav Fomichev via Linux-kernel-mentees
  0 siblings, 0 replies; 24+ messages in thread
From: Stanislav Fomichev via Linux-kernel-mentees @ 2023-07-17 16:46 UTC (permalink / raw)
  To: Anh Tuan Phan; +Cc: daniel, martin.lau, ast, andrii, bpf, linux-kernel-mentees

On Sun, Jul 16, 2023 at 2:42 AM Anh Tuan Phan <tuananhlfc@gmail.com> wrote:
>
>
>
> On 7/11/23 00:18, Stanislav Fomichev wrote:
> > On 07/09, Anh Tuan Phan wrote:
> >> I updated the patch to reflect your suggestion. Thank you!
> >
> > In the future, can you please post a new one with v+1 instead of replying
> > to the old one? Thx!
>
> Will do in the next version.
>
> >
> >> On 7/7/23 23:53, Stanislav Fomichev wrote:
> >>> On 07/07, Anh Tuan Phan wrote:
> >>>>
> >>>>
> >>>> On 7/7/23 01:09, Stanislav Fomichev wrote:
> >>>>> On 07/06, Anh Tuan Phan wrote:
> >>>>>> This commit fixes a few compilation issues when building out of source
> >>>>>> tree. The command that I used to build samples/bpf:
> >>>>>>
> >>>>>> export KBUILD_OUTPUT=/tmp
> >>>>>> make V=1 M=samples/bpf
> >>>>>>
> >>>>>> The compilation failed since it tried to find the header files in the
> >>>>>> wrong places between output directory and source tree directory
> >>>>>>
> >>>>>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
> >>>>>> ---
> >>>>>>  samples/bpf/Makefile        | 8 ++++----
> >>>>>>  samples/bpf/Makefile.target | 2 +-
> >>>>>>  2 files changed, 5 insertions(+), 5 deletions(-)
> >>>>>>
> >>>>>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> >>>>>> index 615f24ebc49c..32469aaa82d5 100644
> >>>>>> --- a/samples/bpf/Makefile
> >>>>>> +++ b/samples/bpf/Makefile
> >>>>>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
> >>>>>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
> >>>>>>  # TPROGS_CFLAGS causes conflicts
> >>>>>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
> >>>>>> -                     -I$(src)/../../tools/include \
> >>>>>> +                     -I$(srctree)/tools/include \
> >>>>>
> >>>>> [..]
> >>>>>
> >>>>>>                       -I$(src)/../../tools/include/uapi \
> >>>>>
> >>>>> Does this $(src) need to be changed as well?
> >>>>
> >>>> I think this line doesn't affect the build. I removed it and it still
> >>>> compiles (after "make -C samples/bpf clean"). I guess xdp_sample_user.c
> >>>> doesn't include any file in tools/include/uapi. Am I missing something
> >>>> or should I remove this line?
> >>>
> >>> You might have these headers installed on your system already if
> >>> it compiles without this part. So I'd keep this part but do
> >>> s/src/srctree/ (and remove ../..).
> >>>
> >>>>>
> >>>>>
> >>>>>>                       -I$(LIBBPF_INCLUDE) \
> >>>>>> -                     -I$(src)/../../tools/testing/selftests/bpf
> >>>>>> +                     -I$(srctree)/tools/testing/selftests/bpf
> >>>>>>
> >>>>>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
> >>>>>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
> >>>>>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
> >>>>>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
> >>>>>> $(src)/xdp_sample_shared.h
> >>>>>>          @echo "  CLANG-BPF " $@
> >>>>>>          $(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
> >>>>>> -                -Wno-compare-distinct-pointer-types -I$(srctree)/include \
> >>>>>> +                -Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
> >>>>>>                  -I$(srctree)/samples/bpf -I$(srctree)/tools/include \
> >>>>>>                  -I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
> >>>>>>                  -c $(filter %.bpf.c,$^) -o $@
> >>>>>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
> >>>>>> xdp_sample.bpf.o
> >>>>>>
> >>>>>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
> >>>>>> skel,$(LINKED_SKELS),$($(skel)-deps)))
> >>>>>>
> >>>>>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
> >>>>>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
> >>>>>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
> >>>>>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
> >>>>>>
> >>>>>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
> >>>>>> index 7621f55e2947..86a454cfb080 100644
> >>>>>> --- a/samples/bpf/Makefile.target
> >>>>>> +++ b/samples/bpf/Makefile.target
> >>>>>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
> >>>>>>                   $(TPROGCFLAGS_$(basetarget).o)
> >>>>>>
> >>>>>>  # $(objtree)/$(obj) for including generated headers from checkin source
> >>>>>> files
> >>>>>
> >>>>> [..]
> >>>>>
> >>>>>> -ifeq ($(KBUILD_EXTMOD),)
> >>>>>> +ifneq ($(KBUILD_EXTMOD),)
> >>>>>
> >>>>> This parts seems to be copy-pasted all over the place in its 'ifeq'
> >>>>> form. What is it doing and why is it needed?
> >>>>>
> >>>>>>  ifdef building_out_of_srctree
> >>>>>>  _tprogc_flags   += -I $(objtree)/$(obj)
> >>>>>>  endif
> >>>>>> --
> >>>>>> 2.34.1
> >>
> >> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
> >> ---
> >>
> >> Change from the original patch
> >>
> >> - Change "-I$(src)/../../tools/include/uapi" to
> >> "-I$(srctree)/tools/include/uapi"
> >>
> >>  samples/bpf/Makefile        | 10 +++++-----
> >>  samples/bpf/Makefile.target |  2 +-
> >>  2 files changed, 6 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> >> index 615f24ebc49c..cfc960b3713a 100644
> >> --- a/samples/bpf/Makefile
> >> +++ b/samples/bpf/Makefile
> >> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
> >>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
> >>  # TPROGS_CFLAGS causes conflicts
> >>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
> >> -                 -I$(src)/../../tools/include \
> >> -                 -I$(src)/../../tools/include/uapi \
> >> +                 -I$(srctree)/tools/include \
> >> +                 -I$(srctree)/tools/include/uapi \
> >>                   -I$(LIBBPF_INCLUDE) \
> >> -                 -I$(src)/../../tools/testing/selftests/bpf
> >> +                 -I$(srctree)/tools/testing/selftests/bpf
> >>
> >>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
> >>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
> >> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
> >>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
> >> $(src)/xdp_sample_shared.h
> >>      @echo "  CLANG-BPF " $@
> >>      $(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
> >> -            -Wno-compare-distinct-pointer-types -I$(srctree)/include \
> >> +            -Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
> >>              -I$(srctree)/samples/bpf -I$(srctree)/tools/include \
> >>              -I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
> >>              -c $(filter %.bpf.c,$^) -o $@
> >> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
> >> xdp_sample.bpf.o
> >>
> >>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
> >> skel,$(LINKED_SKELS),$($(skel)-deps)))
> >>
> >> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
> >> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
> >>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
> >>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
> >>
> >> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
> >> index 7621f55e2947..86a454cfb080 100644
> >> --- a/samples/bpf/Makefile.target
> >> +++ b/samples/bpf/Makefile.target
> >> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
> >>                   $(TPROGCFLAGS_$(basetarget).o)
> >>
> >>  # $(objtree)/$(obj) for including generated headers from checkin source
> >> files
> >
> > [..]
> >
> >> -ifeq ($(KBUILD_EXTMOD),)
> >> +ifneq ($(KBUILD_EXTMOD),)
> >>  ifdef building_out_of_srctree
> >>  _tprogc_flags   += -I $(objtree)/$(obj)
> >>  endif
> >
> > This question left undressed. Can you share more on why this change
> > is needed? Because it looks like it's actually needed for M='' case.
> > IOW, maybe we should add $(objtree)/$(obj) somewhere else?
>
> If it's needed for both cases M='' and M != '', is it better to just
> remove the condition with $(KBUILD_EXTMOD)? FMPOV, -I $(objtree)/$(obj)
> is only needed in case of building_out_of_srctree, no matter what
> KBUILD_EXTMOD.

My guess is that we're missing -I $(objtree)/$(obj) somewhere else.
Depending on what/where it fails, we probably need to add $(objtree)
to some of the $(obj)s?
I'm mostly speculating here, because I see that "ifeq
($(KBUILD_EXTMOD),) + ifdef building_out_of_srctree" pattern being
used in the other places.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v1] samples/bpf: Fix build out of source tree
  2023-07-17 16:46               ` Stanislav Fomichev via Linux-kernel-mentees
@ 2023-07-19 16:13                 ` Anh Tuan Phan
  -1 siblings, 0 replies; 24+ messages in thread
From: Anh Tuan Phan @ 2023-07-19 16:13 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: bpf, ast, daniel, andrii, martin.lau, linux-kernel-mentees



On 7/17/23 23:46, Stanislav Fomichev wrote:
> On Sun, Jul 16, 2023 at 2:42 AM Anh Tuan Phan <tuananhlfc@gmail.com> wrote:
>>
>>
>>
>> On 7/11/23 00:18, Stanislav Fomichev wrote:
>>> On 07/09, Anh Tuan Phan wrote:
>>>> I updated the patch to reflect your suggestion. Thank you!
>>>
>>> In the future, can you please post a new one with v+1 instead of replying
>>> to the old one? Thx!
>>
>> Will do in the next version.
>>
>>>
>>>> On 7/7/23 23:53, Stanislav Fomichev wrote:
>>>>> On 07/07, Anh Tuan Phan wrote:
>>>>>>
>>>>>>
>>>>>> On 7/7/23 01:09, Stanislav Fomichev wrote:
>>>>>>> On 07/06, Anh Tuan Phan wrote:
>>>>>>>> This commit fixes a few compilation issues when building out of source
>>>>>>>> tree. The command that I used to build samples/bpf:
>>>>>>>>
>>>>>>>> export KBUILD_OUTPUT=/tmp
>>>>>>>> make V=1 M=samples/bpf
>>>>>>>>
>>>>>>>> The compilation failed since it tried to find the header files in the
>>>>>>>> wrong places between output directory and source tree directory
>>>>>>>>
>>>>>>>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
>>>>>>>> ---
>>>>>>>>  samples/bpf/Makefile        | 8 ++++----
>>>>>>>>  samples/bpf/Makefile.target | 2 +-
>>>>>>>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>>>>>>>> index 615f24ebc49c..32469aaa82d5 100644
>>>>>>>> --- a/samples/bpf/Makefile
>>>>>>>> +++ b/samples/bpf/Makefile
>>>>>>>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>>>>>>>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
>>>>>>>>  # TPROGS_CFLAGS causes conflicts
>>>>>>>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
>>>>>>>> -                     -I$(src)/../../tools/include \
>>>>>>>> +                     -I$(srctree)/tools/include \
>>>>>>>
>>>>>>> [..]
>>>>>>>
>>>>>>>>                       -I$(src)/../../tools/include/uapi \
>>>>>>>
>>>>>>> Does this $(src) need to be changed as well?
>>>>>>
>>>>>> I think this line doesn't affect the build. I removed it and it still
>>>>>> compiles (after "make -C samples/bpf clean"). I guess xdp_sample_user.c
>>>>>> doesn't include any file in tools/include/uapi. Am I missing something
>>>>>> or should I remove this line?
>>>>>
>>>>> You might have these headers installed on your system already if
>>>>> it compiles without this part. So I'd keep this part but do
>>>>> s/src/srctree/ (and remove ../..).
>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>                       -I$(LIBBPF_INCLUDE) \
>>>>>>>> -                     -I$(src)/../../tools/testing/selftests/bpf
>>>>>>>> +                     -I$(srctree)/tools/testing/selftests/bpf
>>>>>>>>
>>>>>>>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
>>>>>>>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
>>>>>>>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
>>>>>>>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
>>>>>>>> $(src)/xdp_sample_shared.h
>>>>>>>>          @echo "  CLANG-BPF " $@
>>>>>>>>          $(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
>>>>>>>> -                -Wno-compare-distinct-pointer-types -I$(srctree)/include \
>>>>>>>> +                -Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
>>>>>>>>                  -I$(srctree)/samples/bpf -I$(srctree)/tools/include \
>>>>>>>>                  -I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
>>>>>>>>                  -c $(filter %.bpf.c,$^) -o $@
>>>>>>>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
>>>>>>>> xdp_sample.bpf.o
>>>>>>>>
>>>>>>>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
>>>>>>>> skel,$(LINKED_SKELS),$($(skel)-deps)))
>>>>>>>>
>>>>>>>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
>>>>>>>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
>>>>>>>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
>>>>>>>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
>>>>>>>>
>>>>>>>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
>>>>>>>> index 7621f55e2947..86a454cfb080 100644
>>>>>>>> --- a/samples/bpf/Makefile.target
>>>>>>>> +++ b/samples/bpf/Makefile.target
>>>>>>>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
>>>>>>>>                   $(TPROGCFLAGS_$(basetarget).o)
>>>>>>>>
>>>>>>>>  # $(objtree)/$(obj) for including generated headers from checkin source
>>>>>>>> files
>>>>>>>
>>>>>>> [..]
>>>>>>>
>>>>>>>> -ifeq ($(KBUILD_EXTMOD),)
>>>>>>>> +ifneq ($(KBUILD_EXTMOD),)
>>>>>>>
>>>>>>> This parts seems to be copy-pasted all over the place in its 'ifeq'
>>>>>>> form. What is it doing and why is it needed?
>>>>>>>
>>>>>>>>  ifdef building_out_of_srctree
>>>>>>>>  _tprogc_flags   += -I $(objtree)/$(obj)
>>>>>>>>  endif
>>>>>>>> --
>>>>>>>> 2.34.1
>>>>
>>>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
>>>> ---
>>>>
>>>> Change from the original patch
>>>>
>>>> - Change "-I$(src)/../../tools/include/uapi" to
>>>> "-I$(srctree)/tools/include/uapi"
>>>>
>>>>  samples/bpf/Makefile        | 10 +++++-----
>>>>  samples/bpf/Makefile.target |  2 +-
>>>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>>>> index 615f24ebc49c..cfc960b3713a 100644
>>>> --- a/samples/bpf/Makefile
>>>> +++ b/samples/bpf/Makefile
>>>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>>>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
>>>>  # TPROGS_CFLAGS causes conflicts
>>>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
>>>> -                 -I$(src)/../../tools/include \
>>>> -                 -I$(src)/../../tools/include/uapi \
>>>> +                 -I$(srctree)/tools/include \
>>>> +                 -I$(srctree)/tools/include/uapi \
>>>>                   -I$(LIBBPF_INCLUDE) \
>>>> -                 -I$(src)/../../tools/testing/selftests/bpf
>>>> +                 -I$(srctree)/tools/testing/selftests/bpf
>>>>
>>>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
>>>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
>>>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
>>>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
>>>> $(src)/xdp_sample_shared.h
>>>>      @echo "  CLANG-BPF " $@
>>>>      $(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
>>>> -            -Wno-compare-distinct-pointer-types -I$(srctree)/include \
>>>> +            -Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
>>>>              -I$(srctree)/samples/bpf -I$(srctree)/tools/include \
>>>>              -I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
>>>>              -c $(filter %.bpf.c,$^) -o $@
>>>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
>>>> xdp_sample.bpf.o
>>>>
>>>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
>>>> skel,$(LINKED_SKELS),$($(skel)-deps)))
>>>>
>>>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
>>>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
>>>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
>>>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
>>>>
>>>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
>>>> index 7621f55e2947..86a454cfb080 100644
>>>> --- a/samples/bpf/Makefile.target
>>>> +++ b/samples/bpf/Makefile.target
>>>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
>>>>                   $(TPROGCFLAGS_$(basetarget).o)
>>>>
>>>>  # $(objtree)/$(obj) for including generated headers from checkin source
>>>> files
>>>
>>> [..]
>>>
>>>> -ifeq ($(KBUILD_EXTMOD),)
>>>> +ifneq ($(KBUILD_EXTMOD),)
>>>>  ifdef building_out_of_srctree
>>>>  _tprogc_flags   += -I $(objtree)/$(obj)
>>>>  endif
>>>
>>> This question left undressed. Can you share more on why this change
>>> is needed? Because it looks like it's actually needed for M='' case.
>>> IOW, maybe we should add $(objtree)/$(obj) somewhere else?
>>
>> If it's needed for both cases M='' and M != '', is it better to just
>> remove the condition with $(KBUILD_EXTMOD)? FMPOV, -I $(objtree)/$(obj)
>> is only needed in case of building_out_of_srctree, no matter what
>> KBUILD_EXTMOD.
> 
> My guess is that we're missing -I $(objtree)/$(obj) somewhere else.
> Depending on what/where it fails, we probably need to add $(objtree)
> to some of the $(obj)s?
> I'm mostly speculating here, because I see that "ifeq
> ($(KBUILD_EXTMOD),) + ifdef building_out_of_srctree" pattern being
> used in the other places.

It turns out that your guess is right. "-I $(objtree)/$(obj)" is only
needed for the following objects: xdp_redirect_map_multi_user,
xdp_redirect_cpu_user, xdp_redirect_map_user, xdp_redirect_user,
xdp_monitor_user and xdp_router_ipv4_user. There is a variable to add
flags for gcc named $(TPROGCFLAGS_$(basetarget).o) but it has not been
set. My proposed fix is to set the following variables:

+TPROGCFLAGS_xdp_redirect_map_multi_user.o += -I$(objtree)/$(obj)
+TPROGCFLAGS_xdp_redirect_cpu_user.o += -I$(objtree)/$(obj)
+TPROGCFLAGS_xdp_redirect_map_user.o += -I$(objtree)/$(obj)
+TPROGCFLAGS_xdp_redirect_user.o += -I$(objtree)/$(obj)
+TPROGCFLAGS_xdp_monitor_user.o += -I$(objtree)/$(obj)
+TPROGCFLAGS_xdp_router_ipv4_user.o += -I$(objtree)/$(obj)

It works with the original "ifeq
> ($(KBUILD_EXTMOD),) + ifdef building_out_of_srctree" pattern. What do
you think about my proposed fix?

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v1] samples/bpf: Fix build out of source tree
@ 2023-07-19 16:13                 ` Anh Tuan Phan
  0 siblings, 0 replies; 24+ messages in thread
From: Anh Tuan Phan @ 2023-07-19 16:13 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: daniel, martin.lau, ast, andrii, bpf, linux-kernel-mentees



On 7/17/23 23:46, Stanislav Fomichev wrote:
> On Sun, Jul 16, 2023 at 2:42 AM Anh Tuan Phan <tuananhlfc@gmail.com> wrote:
>>
>>
>>
>> On 7/11/23 00:18, Stanislav Fomichev wrote:
>>> On 07/09, Anh Tuan Phan wrote:
>>>> I updated the patch to reflect your suggestion. Thank you!
>>>
>>> In the future, can you please post a new one with v+1 instead of replying
>>> to the old one? Thx!
>>
>> Will do in the next version.
>>
>>>
>>>> On 7/7/23 23:53, Stanislav Fomichev wrote:
>>>>> On 07/07, Anh Tuan Phan wrote:
>>>>>>
>>>>>>
>>>>>> On 7/7/23 01:09, Stanislav Fomichev wrote:
>>>>>>> On 07/06, Anh Tuan Phan wrote:
>>>>>>>> This commit fixes a few compilation issues when building out of source
>>>>>>>> tree. The command that I used to build samples/bpf:
>>>>>>>>
>>>>>>>> export KBUILD_OUTPUT=/tmp
>>>>>>>> make V=1 M=samples/bpf
>>>>>>>>
>>>>>>>> The compilation failed since it tried to find the header files in the
>>>>>>>> wrong places between output directory and source tree directory
>>>>>>>>
>>>>>>>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
>>>>>>>> ---
>>>>>>>>  samples/bpf/Makefile        | 8 ++++----
>>>>>>>>  samples/bpf/Makefile.target | 2 +-
>>>>>>>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>>>>>>>> index 615f24ebc49c..32469aaa82d5 100644
>>>>>>>> --- a/samples/bpf/Makefile
>>>>>>>> +++ b/samples/bpf/Makefile
>>>>>>>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>>>>>>>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
>>>>>>>>  # TPROGS_CFLAGS causes conflicts
>>>>>>>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
>>>>>>>> -                     -I$(src)/../../tools/include \
>>>>>>>> +                     -I$(srctree)/tools/include \
>>>>>>>
>>>>>>> [..]
>>>>>>>
>>>>>>>>                       -I$(src)/../../tools/include/uapi \
>>>>>>>
>>>>>>> Does this $(src) need to be changed as well?
>>>>>>
>>>>>> I think this line doesn't affect the build. I removed it and it still
>>>>>> compiles (after "make -C samples/bpf clean"). I guess xdp_sample_user.c
>>>>>> doesn't include any file in tools/include/uapi. Am I missing something
>>>>>> or should I remove this line?
>>>>>
>>>>> You might have these headers installed on your system already if
>>>>> it compiles without this part. So I'd keep this part but do
>>>>> s/src/srctree/ (and remove ../..).
>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>                       -I$(LIBBPF_INCLUDE) \
>>>>>>>> -                     -I$(src)/../../tools/testing/selftests/bpf
>>>>>>>> +                     -I$(srctree)/tools/testing/selftests/bpf
>>>>>>>>
>>>>>>>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
>>>>>>>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
>>>>>>>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
>>>>>>>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
>>>>>>>> $(src)/xdp_sample_shared.h
>>>>>>>>          @echo "  CLANG-BPF " $@
>>>>>>>>          $(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
>>>>>>>> -                -Wno-compare-distinct-pointer-types -I$(srctree)/include \
>>>>>>>> +                -Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
>>>>>>>>                  -I$(srctree)/samples/bpf -I$(srctree)/tools/include \
>>>>>>>>                  -I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
>>>>>>>>                  -c $(filter %.bpf.c,$^) -o $@
>>>>>>>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
>>>>>>>> xdp_sample.bpf.o
>>>>>>>>
>>>>>>>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
>>>>>>>> skel,$(LINKED_SKELS),$($(skel)-deps)))
>>>>>>>>
>>>>>>>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
>>>>>>>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
>>>>>>>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
>>>>>>>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
>>>>>>>>
>>>>>>>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
>>>>>>>> index 7621f55e2947..86a454cfb080 100644
>>>>>>>> --- a/samples/bpf/Makefile.target
>>>>>>>> +++ b/samples/bpf/Makefile.target
>>>>>>>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
>>>>>>>>                   $(TPROGCFLAGS_$(basetarget).o)
>>>>>>>>
>>>>>>>>  # $(objtree)/$(obj) for including generated headers from checkin source
>>>>>>>> files
>>>>>>>
>>>>>>> [..]
>>>>>>>
>>>>>>>> -ifeq ($(KBUILD_EXTMOD),)
>>>>>>>> +ifneq ($(KBUILD_EXTMOD),)
>>>>>>>
>>>>>>> This parts seems to be copy-pasted all over the place in its 'ifeq'
>>>>>>> form. What is it doing and why is it needed?
>>>>>>>
>>>>>>>>  ifdef building_out_of_srctree
>>>>>>>>  _tprogc_flags   += -I $(objtree)/$(obj)
>>>>>>>>  endif
>>>>>>>> --
>>>>>>>> 2.34.1
>>>>
>>>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
>>>> ---
>>>>
>>>> Change from the original patch
>>>>
>>>> - Change "-I$(src)/../../tools/include/uapi" to
>>>> "-I$(srctree)/tools/include/uapi"
>>>>
>>>>  samples/bpf/Makefile        | 10 +++++-----
>>>>  samples/bpf/Makefile.target |  2 +-
>>>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>>>> index 615f24ebc49c..cfc960b3713a 100644
>>>> --- a/samples/bpf/Makefile
>>>> +++ b/samples/bpf/Makefile
>>>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>>>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
>>>>  # TPROGS_CFLAGS causes conflicts
>>>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
>>>> -                 -I$(src)/../../tools/include \
>>>> -                 -I$(src)/../../tools/include/uapi \
>>>> +                 -I$(srctree)/tools/include \
>>>> +                 -I$(srctree)/tools/include/uapi \
>>>>                   -I$(LIBBPF_INCLUDE) \
>>>> -                 -I$(src)/../../tools/testing/selftests/bpf
>>>> +                 -I$(srctree)/tools/testing/selftests/bpf
>>>>
>>>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
>>>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
>>>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
>>>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
>>>> $(src)/xdp_sample_shared.h
>>>>      @echo "  CLANG-BPF " $@
>>>>      $(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
>>>> -            -Wno-compare-distinct-pointer-types -I$(srctree)/include \
>>>> +            -Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
>>>>              -I$(srctree)/samples/bpf -I$(srctree)/tools/include \
>>>>              -I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
>>>>              -c $(filter %.bpf.c,$^) -o $@
>>>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
>>>> xdp_sample.bpf.o
>>>>
>>>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
>>>> skel,$(LINKED_SKELS),$($(skel)-deps)))
>>>>
>>>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
>>>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
>>>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
>>>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
>>>>
>>>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
>>>> index 7621f55e2947..86a454cfb080 100644
>>>> --- a/samples/bpf/Makefile.target
>>>> +++ b/samples/bpf/Makefile.target
>>>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
>>>>                   $(TPROGCFLAGS_$(basetarget).o)
>>>>
>>>>  # $(objtree)/$(obj) for including generated headers from checkin source
>>>> files
>>>
>>> [..]
>>>
>>>> -ifeq ($(KBUILD_EXTMOD),)
>>>> +ifneq ($(KBUILD_EXTMOD),)
>>>>  ifdef building_out_of_srctree
>>>>  _tprogc_flags   += -I $(objtree)/$(obj)
>>>>  endif
>>>
>>> This question left undressed. Can you share more on why this change
>>> is needed? Because it looks like it's actually needed for M='' case.
>>> IOW, maybe we should add $(objtree)/$(obj) somewhere else?
>>
>> If it's needed for both cases M='' and M != '', is it better to just
>> remove the condition with $(KBUILD_EXTMOD)? FMPOV, -I $(objtree)/$(obj)
>> is only needed in case of building_out_of_srctree, no matter what
>> KBUILD_EXTMOD.
> 
> My guess is that we're missing -I $(objtree)/$(obj) somewhere else.
> Depending on what/where it fails, we probably need to add $(objtree)
> to some of the $(obj)s?
> I'm mostly speculating here, because I see that "ifeq
> ($(KBUILD_EXTMOD),) + ifdef building_out_of_srctree" pattern being
> used in the other places.

It turns out that your guess is right. "-I $(objtree)/$(obj)" is only
needed for the following objects: xdp_redirect_map_multi_user,
xdp_redirect_cpu_user, xdp_redirect_map_user, xdp_redirect_user,
xdp_monitor_user and xdp_router_ipv4_user. There is a variable to add
flags for gcc named $(TPROGCFLAGS_$(basetarget).o) but it has not been
set. My proposed fix is to set the following variables:

+TPROGCFLAGS_xdp_redirect_map_multi_user.o += -I$(objtree)/$(obj)
+TPROGCFLAGS_xdp_redirect_cpu_user.o += -I$(objtree)/$(obj)
+TPROGCFLAGS_xdp_redirect_map_user.o += -I$(objtree)/$(obj)
+TPROGCFLAGS_xdp_redirect_user.o += -I$(objtree)/$(obj)
+TPROGCFLAGS_xdp_monitor_user.o += -I$(objtree)/$(obj)
+TPROGCFLAGS_xdp_router_ipv4_user.o += -I$(objtree)/$(obj)

It works with the original "ifeq
> ($(KBUILD_EXTMOD),) + ifdef building_out_of_srctree" pattern. What do
you think about my proposed fix?
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v1] samples/bpf: Fix build out of source tree
  2023-07-19 16:13                 ` Anh Tuan Phan
@ 2023-07-20 17:05                   ` Stanislav Fomichev via Linux-kernel-mentees
  -1 siblings, 0 replies; 24+ messages in thread
From: Stanislav Fomichev @ 2023-07-20 17:05 UTC (permalink / raw)
  To: Anh Tuan Phan; +Cc: bpf, ast, daniel, andrii, martin.lau, linux-kernel-mentees

On Wed, Jul 19, 2023 at 9:13 AM Anh Tuan Phan <tuananhlfc@gmail.com> wrote:
>
>
>
> On 7/17/23 23:46, Stanislav Fomichev wrote:
> > On Sun, Jul 16, 2023 at 2:42 AM Anh Tuan Phan <tuananhlfc@gmail.com> wrote:
> >>
> >>
> >>
> >> On 7/11/23 00:18, Stanislav Fomichev wrote:
> >>> On 07/09, Anh Tuan Phan wrote:
> >>>> I updated the patch to reflect your suggestion. Thank you!
> >>>
> >>> In the future, can you please post a new one with v+1 instead of replying
> >>> to the old one? Thx!
> >>
> >> Will do in the next version.
> >>
> >>>
> >>>> On 7/7/23 23:53, Stanislav Fomichev wrote:
> >>>>> On 07/07, Anh Tuan Phan wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 7/7/23 01:09, Stanislav Fomichev wrote:
> >>>>>>> On 07/06, Anh Tuan Phan wrote:
> >>>>>>>> This commit fixes a few compilation issues when building out of source
> >>>>>>>> tree. The command that I used to build samples/bpf:
> >>>>>>>>
> >>>>>>>> export KBUILD_OUTPUT=/tmp
> >>>>>>>> make V=1 M=samples/bpf
> >>>>>>>>
> >>>>>>>> The compilation failed since it tried to find the header files in the
> >>>>>>>> wrong places between output directory and source tree directory
> >>>>>>>>
> >>>>>>>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
> >>>>>>>> ---
> >>>>>>>>  samples/bpf/Makefile        | 8 ++++----
> >>>>>>>>  samples/bpf/Makefile.target | 2 +-
> >>>>>>>>  2 files changed, 5 insertions(+), 5 deletions(-)
> >>>>>>>>
> >>>>>>>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> >>>>>>>> index 615f24ebc49c..32469aaa82d5 100644
> >>>>>>>> --- a/samples/bpf/Makefile
> >>>>>>>> +++ b/samples/bpf/Makefile
> >>>>>>>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
> >>>>>>>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
> >>>>>>>>  # TPROGS_CFLAGS causes conflicts
> >>>>>>>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
> >>>>>>>> -                     -I$(src)/../../tools/include \
> >>>>>>>> +                     -I$(srctree)/tools/include \
> >>>>>>>
> >>>>>>> [..]
> >>>>>>>
> >>>>>>>>                       -I$(src)/../../tools/include/uapi \
> >>>>>>>
> >>>>>>> Does this $(src) need to be changed as well?
> >>>>>>
> >>>>>> I think this line doesn't affect the build. I removed it and it still
> >>>>>> compiles (after "make -C samples/bpf clean"). I guess xdp_sample_user.c
> >>>>>> doesn't include any file in tools/include/uapi. Am I missing something
> >>>>>> or should I remove this line?
> >>>>>
> >>>>> You might have these headers installed on your system already if
> >>>>> it compiles without this part. So I'd keep this part but do
> >>>>> s/src/srctree/ (and remove ../..).
> >>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>>                       -I$(LIBBPF_INCLUDE) \
> >>>>>>>> -                     -I$(src)/../../tools/testing/selftests/bpf
> >>>>>>>> +                     -I$(srctree)/tools/testing/selftests/bpf
> >>>>>>>>
> >>>>>>>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
> >>>>>>>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
> >>>>>>>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
> >>>>>>>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
> >>>>>>>> $(src)/xdp_sample_shared.h
> >>>>>>>>          @echo "  CLANG-BPF " $@
> >>>>>>>>          $(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
> >>>>>>>> -                -Wno-compare-distinct-pointer-types -I$(srctree)/include \
> >>>>>>>> +                -Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
> >>>>>>>>                  -I$(srctree)/samples/bpf -I$(srctree)/tools/include \
> >>>>>>>>                  -I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
> >>>>>>>>                  -c $(filter %.bpf.c,$^) -o $@
> >>>>>>>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
> >>>>>>>> xdp_sample.bpf.o
> >>>>>>>>
> >>>>>>>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
> >>>>>>>> skel,$(LINKED_SKELS),$($(skel)-deps)))
> >>>>>>>>
> >>>>>>>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
> >>>>>>>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
> >>>>>>>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
> >>>>>>>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
> >>>>>>>>
> >>>>>>>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
> >>>>>>>> index 7621f55e2947..86a454cfb080 100644
> >>>>>>>> --- a/samples/bpf/Makefile.target
> >>>>>>>> +++ b/samples/bpf/Makefile.target
> >>>>>>>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
> >>>>>>>>                   $(TPROGCFLAGS_$(basetarget).o)
> >>>>>>>>
> >>>>>>>>  # $(objtree)/$(obj) for including generated headers from checkin source
> >>>>>>>> files
> >>>>>>>
> >>>>>>> [..]
> >>>>>>>
> >>>>>>>> -ifeq ($(KBUILD_EXTMOD),)
> >>>>>>>> +ifneq ($(KBUILD_EXTMOD),)
> >>>>>>>
> >>>>>>> This parts seems to be copy-pasted all over the place in its 'ifeq'
> >>>>>>> form. What is it doing and why is it needed?
> >>>>>>>
> >>>>>>>>  ifdef building_out_of_srctree
> >>>>>>>>  _tprogc_flags   += -I $(objtree)/$(obj)
> >>>>>>>>  endif
> >>>>>>>> --
> >>>>>>>> 2.34.1
> >>>>
> >>>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
> >>>> ---
> >>>>
> >>>> Change from the original patch
> >>>>
> >>>> - Change "-I$(src)/../../tools/include/uapi" to
> >>>> "-I$(srctree)/tools/include/uapi"
> >>>>
> >>>>  samples/bpf/Makefile        | 10 +++++-----
> >>>>  samples/bpf/Makefile.target |  2 +-
> >>>>  2 files changed, 6 insertions(+), 6 deletions(-)
> >>>>
> >>>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> >>>> index 615f24ebc49c..cfc960b3713a 100644
> >>>> --- a/samples/bpf/Makefile
> >>>> +++ b/samples/bpf/Makefile
> >>>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
> >>>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
> >>>>  # TPROGS_CFLAGS causes conflicts
> >>>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
> >>>> -                 -I$(src)/../../tools/include \
> >>>> -                 -I$(src)/../../tools/include/uapi \
> >>>> +                 -I$(srctree)/tools/include \
> >>>> +                 -I$(srctree)/tools/include/uapi \
> >>>>                   -I$(LIBBPF_INCLUDE) \
> >>>> -                 -I$(src)/../../tools/testing/selftests/bpf
> >>>> +                 -I$(srctree)/tools/testing/selftests/bpf
> >>>>
> >>>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
> >>>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
> >>>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
> >>>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
> >>>> $(src)/xdp_sample_shared.h
> >>>>      @echo "  CLANG-BPF " $@
> >>>>      $(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
> >>>> -            -Wno-compare-distinct-pointer-types -I$(srctree)/include \
> >>>> +            -Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
> >>>>              -I$(srctree)/samples/bpf -I$(srctree)/tools/include \
> >>>>              -I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
> >>>>              -c $(filter %.bpf.c,$^) -o $@
> >>>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
> >>>> xdp_sample.bpf.o
> >>>>
> >>>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
> >>>> skel,$(LINKED_SKELS),$($(skel)-deps)))
> >>>>
> >>>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
> >>>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
> >>>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
> >>>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
> >>>>
> >>>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
> >>>> index 7621f55e2947..86a454cfb080 100644
> >>>> --- a/samples/bpf/Makefile.target
> >>>> +++ b/samples/bpf/Makefile.target
> >>>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
> >>>>                   $(TPROGCFLAGS_$(basetarget).o)
> >>>>
> >>>>  # $(objtree)/$(obj) for including generated headers from checkin source
> >>>> files
> >>>
> >>> [..]
> >>>
> >>>> -ifeq ($(KBUILD_EXTMOD),)
> >>>> +ifneq ($(KBUILD_EXTMOD),)
> >>>>  ifdef building_out_of_srctree
> >>>>  _tprogc_flags   += -I $(objtree)/$(obj)
> >>>>  endif
> >>>
> >>> This question left undressed. Can you share more on why this change
> >>> is needed? Because it looks like it's actually needed for M='' case.
> >>> IOW, maybe we should add $(objtree)/$(obj) somewhere else?
> >>
> >> If it's needed for both cases M='' and M != '', is it better to just
> >> remove the condition with $(KBUILD_EXTMOD)? FMPOV, -I $(objtree)/$(obj)
> >> is only needed in case of building_out_of_srctree, no matter what
> >> KBUILD_EXTMOD.
> >
> > My guess is that we're missing -I $(objtree)/$(obj) somewhere else.
> > Depending on what/where it fails, we probably need to add $(objtree)
> > to some of the $(obj)s?
> > I'm mostly speculating here, because I see that "ifeq
> > ($(KBUILD_EXTMOD),) + ifdef building_out_of_srctree" pattern being
> > used in the other places.
>
> It turns out that your guess is right. "-I $(objtree)/$(obj)" is only
> needed for the following objects: xdp_redirect_map_multi_user,
> xdp_redirect_cpu_user, xdp_redirect_map_user, xdp_redirect_user,
> xdp_monitor_user and xdp_router_ipv4_user. There is a variable to add
> flags for gcc named $(TPROGCFLAGS_$(basetarget).o) but it has not been
> set. My proposed fix is to set the following variables:

Looks like these are the tests that use bpf skeleton.

> +TPROGCFLAGS_xdp_redirect_map_multi_user.o += -I$(objtree)/$(obj)
> +TPROGCFLAGS_xdp_redirect_cpu_user.o += -I$(objtree)/$(obj)
> +TPROGCFLAGS_xdp_redirect_map_user.o += -I$(objtree)/$(obj)
> +TPROGCFLAGS_xdp_redirect_user.o += -I$(objtree)/$(obj)
> +TPROGCFLAGS_xdp_monitor_user.o += -I$(objtree)/$(obj)
> +TPROGCFLAGS_xdp_router_ipv4_user.o += -I$(objtree)/$(obj)
>
> It works with the original "ifeq
> > ($(KBUILD_EXTMOD),) + ifdef building_out_of_srctree" pattern. What do
> you think about my proposed fix?

Maybe we should just unconditionally add "-I $(objtree)/$(obj)" to
_tprogc_flags ?

And then we can drop that whole part:
ifeq ($(KBUILD_EXTMOD),)
ifdef building_out_of_srctree
_tprogc_flags   += -I $(objtree)/$(obj)
endif
endif

IOW, replace that $(TPROGCFLAGS_$(basetarget).o) part with -I
$(objtree)/$(obj) and call it a day?
I honestly have no clue why it's this complicated and I doubt anybody
at this point remembers...

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v1] samples/bpf: Fix build out of source tree
@ 2023-07-20 17:05                   ` Stanislav Fomichev via Linux-kernel-mentees
  0 siblings, 0 replies; 24+ messages in thread
From: Stanislav Fomichev via Linux-kernel-mentees @ 2023-07-20 17:05 UTC (permalink / raw)
  To: Anh Tuan Phan; +Cc: daniel, martin.lau, ast, andrii, bpf, linux-kernel-mentees

On Wed, Jul 19, 2023 at 9:13 AM Anh Tuan Phan <tuananhlfc@gmail.com> wrote:
>
>
>
> On 7/17/23 23:46, Stanislav Fomichev wrote:
> > On Sun, Jul 16, 2023 at 2:42 AM Anh Tuan Phan <tuananhlfc@gmail.com> wrote:
> >>
> >>
> >>
> >> On 7/11/23 00:18, Stanislav Fomichev wrote:
> >>> On 07/09, Anh Tuan Phan wrote:
> >>>> I updated the patch to reflect your suggestion. Thank you!
> >>>
> >>> In the future, can you please post a new one with v+1 instead of replying
> >>> to the old one? Thx!
> >>
> >> Will do in the next version.
> >>
> >>>
> >>>> On 7/7/23 23:53, Stanislav Fomichev wrote:
> >>>>> On 07/07, Anh Tuan Phan wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 7/7/23 01:09, Stanislav Fomichev wrote:
> >>>>>>> On 07/06, Anh Tuan Phan wrote:
> >>>>>>>> This commit fixes a few compilation issues when building out of source
> >>>>>>>> tree. The command that I used to build samples/bpf:
> >>>>>>>>
> >>>>>>>> export KBUILD_OUTPUT=/tmp
> >>>>>>>> make V=1 M=samples/bpf
> >>>>>>>>
> >>>>>>>> The compilation failed since it tried to find the header files in the
> >>>>>>>> wrong places between output directory and source tree directory
> >>>>>>>>
> >>>>>>>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
> >>>>>>>> ---
> >>>>>>>>  samples/bpf/Makefile        | 8 ++++----
> >>>>>>>>  samples/bpf/Makefile.target | 2 +-
> >>>>>>>>  2 files changed, 5 insertions(+), 5 deletions(-)
> >>>>>>>>
> >>>>>>>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> >>>>>>>> index 615f24ebc49c..32469aaa82d5 100644
> >>>>>>>> --- a/samples/bpf/Makefile
> >>>>>>>> +++ b/samples/bpf/Makefile
> >>>>>>>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
> >>>>>>>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
> >>>>>>>>  # TPROGS_CFLAGS causes conflicts
> >>>>>>>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
> >>>>>>>> -                     -I$(src)/../../tools/include \
> >>>>>>>> +                     -I$(srctree)/tools/include \
> >>>>>>>
> >>>>>>> [..]
> >>>>>>>
> >>>>>>>>                       -I$(src)/../../tools/include/uapi \
> >>>>>>>
> >>>>>>> Does this $(src) need to be changed as well?
> >>>>>>
> >>>>>> I think this line doesn't affect the build. I removed it and it still
> >>>>>> compiles (after "make -C samples/bpf clean"). I guess xdp_sample_user.c
> >>>>>> doesn't include any file in tools/include/uapi. Am I missing something
> >>>>>> or should I remove this line?
> >>>>>
> >>>>> You might have these headers installed on your system already if
> >>>>> it compiles without this part. So I'd keep this part but do
> >>>>> s/src/srctree/ (and remove ../..).
> >>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>>                       -I$(LIBBPF_INCLUDE) \
> >>>>>>>> -                     -I$(src)/../../tools/testing/selftests/bpf
> >>>>>>>> +                     -I$(srctree)/tools/testing/selftests/bpf
> >>>>>>>>
> >>>>>>>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
> >>>>>>>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
> >>>>>>>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
> >>>>>>>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
> >>>>>>>> $(src)/xdp_sample_shared.h
> >>>>>>>>          @echo "  CLANG-BPF " $@
> >>>>>>>>          $(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
> >>>>>>>> -                -Wno-compare-distinct-pointer-types -I$(srctree)/include \
> >>>>>>>> +                -Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
> >>>>>>>>                  -I$(srctree)/samples/bpf -I$(srctree)/tools/include \
> >>>>>>>>                  -I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
> >>>>>>>>                  -c $(filter %.bpf.c,$^) -o $@
> >>>>>>>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
> >>>>>>>> xdp_sample.bpf.o
> >>>>>>>>
> >>>>>>>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
> >>>>>>>> skel,$(LINKED_SKELS),$($(skel)-deps)))
> >>>>>>>>
> >>>>>>>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
> >>>>>>>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
> >>>>>>>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
> >>>>>>>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
> >>>>>>>>
> >>>>>>>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
> >>>>>>>> index 7621f55e2947..86a454cfb080 100644
> >>>>>>>> --- a/samples/bpf/Makefile.target
> >>>>>>>> +++ b/samples/bpf/Makefile.target
> >>>>>>>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
> >>>>>>>>                   $(TPROGCFLAGS_$(basetarget).o)
> >>>>>>>>
> >>>>>>>>  # $(objtree)/$(obj) for including generated headers from checkin source
> >>>>>>>> files
> >>>>>>>
> >>>>>>> [..]
> >>>>>>>
> >>>>>>>> -ifeq ($(KBUILD_EXTMOD),)
> >>>>>>>> +ifneq ($(KBUILD_EXTMOD),)
> >>>>>>>
> >>>>>>> This parts seems to be copy-pasted all over the place in its 'ifeq'
> >>>>>>> form. What is it doing and why is it needed?
> >>>>>>>
> >>>>>>>>  ifdef building_out_of_srctree
> >>>>>>>>  _tprogc_flags   += -I $(objtree)/$(obj)
> >>>>>>>>  endif
> >>>>>>>> --
> >>>>>>>> 2.34.1
> >>>>
> >>>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
> >>>> ---
> >>>>
> >>>> Change from the original patch
> >>>>
> >>>> - Change "-I$(src)/../../tools/include/uapi" to
> >>>> "-I$(srctree)/tools/include/uapi"
> >>>>
> >>>>  samples/bpf/Makefile        | 10 +++++-----
> >>>>  samples/bpf/Makefile.target |  2 +-
> >>>>  2 files changed, 6 insertions(+), 6 deletions(-)
> >>>>
> >>>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> >>>> index 615f24ebc49c..cfc960b3713a 100644
> >>>> --- a/samples/bpf/Makefile
> >>>> +++ b/samples/bpf/Makefile
> >>>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
> >>>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
> >>>>  # TPROGS_CFLAGS causes conflicts
> >>>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
> >>>> -                 -I$(src)/../../tools/include \
> >>>> -                 -I$(src)/../../tools/include/uapi \
> >>>> +                 -I$(srctree)/tools/include \
> >>>> +                 -I$(srctree)/tools/include/uapi \
> >>>>                   -I$(LIBBPF_INCLUDE) \
> >>>> -                 -I$(src)/../../tools/testing/selftests/bpf
> >>>> +                 -I$(srctree)/tools/testing/selftests/bpf
> >>>>
> >>>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
> >>>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
> >>>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
> >>>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
> >>>> $(src)/xdp_sample_shared.h
> >>>>      @echo "  CLANG-BPF " $@
> >>>>      $(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
> >>>> -            -Wno-compare-distinct-pointer-types -I$(srctree)/include \
> >>>> +            -Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
> >>>>              -I$(srctree)/samples/bpf -I$(srctree)/tools/include \
> >>>>              -I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
> >>>>              -c $(filter %.bpf.c,$^) -o $@
> >>>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
> >>>> xdp_sample.bpf.o
> >>>>
> >>>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
> >>>> skel,$(LINKED_SKELS),$($(skel)-deps)))
> >>>>
> >>>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
> >>>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
> >>>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
> >>>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
> >>>>
> >>>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
> >>>> index 7621f55e2947..86a454cfb080 100644
> >>>> --- a/samples/bpf/Makefile.target
> >>>> +++ b/samples/bpf/Makefile.target
> >>>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
> >>>>                   $(TPROGCFLAGS_$(basetarget).o)
> >>>>
> >>>>  # $(objtree)/$(obj) for including generated headers from checkin source
> >>>> files
> >>>
> >>> [..]
> >>>
> >>>> -ifeq ($(KBUILD_EXTMOD),)
> >>>> +ifneq ($(KBUILD_EXTMOD),)
> >>>>  ifdef building_out_of_srctree
> >>>>  _tprogc_flags   += -I $(objtree)/$(obj)
> >>>>  endif
> >>>
> >>> This question left undressed. Can you share more on why this change
> >>> is needed? Because it looks like it's actually needed for M='' case.
> >>> IOW, maybe we should add $(objtree)/$(obj) somewhere else?
> >>
> >> If it's needed for both cases M='' and M != '', is it better to just
> >> remove the condition with $(KBUILD_EXTMOD)? FMPOV, -I $(objtree)/$(obj)
> >> is only needed in case of building_out_of_srctree, no matter what
> >> KBUILD_EXTMOD.
> >
> > My guess is that we're missing -I $(objtree)/$(obj) somewhere else.
> > Depending on what/where it fails, we probably need to add $(objtree)
> > to some of the $(obj)s?
> > I'm mostly speculating here, because I see that "ifeq
> > ($(KBUILD_EXTMOD),) + ifdef building_out_of_srctree" pattern being
> > used in the other places.
>
> It turns out that your guess is right. "-I $(objtree)/$(obj)" is only
> needed for the following objects: xdp_redirect_map_multi_user,
> xdp_redirect_cpu_user, xdp_redirect_map_user, xdp_redirect_user,
> xdp_monitor_user and xdp_router_ipv4_user. There is a variable to add
> flags for gcc named $(TPROGCFLAGS_$(basetarget).o) but it has not been
> set. My proposed fix is to set the following variables:

Looks like these are the tests that use bpf skeleton.

> +TPROGCFLAGS_xdp_redirect_map_multi_user.o += -I$(objtree)/$(obj)
> +TPROGCFLAGS_xdp_redirect_cpu_user.o += -I$(objtree)/$(obj)
> +TPROGCFLAGS_xdp_redirect_map_user.o += -I$(objtree)/$(obj)
> +TPROGCFLAGS_xdp_redirect_user.o += -I$(objtree)/$(obj)
> +TPROGCFLAGS_xdp_monitor_user.o += -I$(objtree)/$(obj)
> +TPROGCFLAGS_xdp_router_ipv4_user.o += -I$(objtree)/$(obj)
>
> It works with the original "ifeq
> > ($(KBUILD_EXTMOD),) + ifdef building_out_of_srctree" pattern. What do
> you think about my proposed fix?

Maybe we should just unconditionally add "-I $(objtree)/$(obj)" to
_tprogc_flags ?

And then we can drop that whole part:
ifeq ($(KBUILD_EXTMOD),)
ifdef building_out_of_srctree
_tprogc_flags   += -I $(objtree)/$(obj)
endif
endif

IOW, replace that $(TPROGCFLAGS_$(basetarget).o) part with -I
$(objtree)/$(obj) and call it a day?
I honestly have no clue why it's this complicated and I doubt anybody
at this point remembers...
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v1] samples/bpf: Fix build out of source tree
  2023-07-20 17:05                   ` Stanislav Fomichev via Linux-kernel-mentees
@ 2023-07-21  1:05                     ` Anh Tuan Phan
  -1 siblings, 0 replies; 24+ messages in thread
From: Anh Tuan Phan @ 2023-07-21  1:05 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: bpf, ast, daniel, andrii, martin.lau, linux-kernel-mentees



On 7/21/23 00:05, Stanislav Fomichev wrote:
> On Wed, Jul 19, 2023 at 9:13 AM Anh Tuan Phan <tuananhlfc@gmail.com> wrote:
>>
>>
>>
>> On 7/17/23 23:46, Stanislav Fomichev wrote:
>>> On Sun, Jul 16, 2023 at 2:42 AM Anh Tuan Phan <tuananhlfc@gmail.com> wrote:
>>>>
>>>>
>>>>
>>>> On 7/11/23 00:18, Stanislav Fomichev wrote:
>>>>> On 07/09, Anh Tuan Phan wrote:
>>>>>> I updated the patch to reflect your suggestion. Thank you!
>>>>>
>>>>> In the future, can you please post a new one with v+1 instead of replying
>>>>> to the old one? Thx!
>>>>
>>>> Will do in the next version.
>>>>
>>>>>
>>>>>> On 7/7/23 23:53, Stanislav Fomichev wrote:
>>>>>>> On 07/07, Anh Tuan Phan wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 7/7/23 01:09, Stanislav Fomichev wrote:
>>>>>>>>> On 07/06, Anh Tuan Phan wrote:
>>>>>>>>>> This commit fixes a few compilation issues when building out of source
>>>>>>>>>> tree. The command that I used to build samples/bpf:
>>>>>>>>>>
>>>>>>>>>> export KBUILD_OUTPUT=/tmp
>>>>>>>>>> make V=1 M=samples/bpf
>>>>>>>>>>
>>>>>>>>>> The compilation failed since it tried to find the header files in the
>>>>>>>>>> wrong places between output directory and source tree directory
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
>>>>>>>>>> ---
>>>>>>>>>>  samples/bpf/Makefile        | 8 ++++----
>>>>>>>>>>  samples/bpf/Makefile.target | 2 +-
>>>>>>>>>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>>>>>>>>>> index 615f24ebc49c..32469aaa82d5 100644
>>>>>>>>>> --- a/samples/bpf/Makefile
>>>>>>>>>> +++ b/samples/bpf/Makefile
>>>>>>>>>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>>>>>>>>>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
>>>>>>>>>>  # TPROGS_CFLAGS causes conflicts
>>>>>>>>>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
>>>>>>>>>> -                     -I$(src)/../../tools/include \
>>>>>>>>>> +                     -I$(srctree)/tools/include \
>>>>>>>>>
>>>>>>>>> [..]
>>>>>>>>>
>>>>>>>>>>                       -I$(src)/../../tools/include/uapi \
>>>>>>>>>
>>>>>>>>> Does this $(src) need to be changed as well?
>>>>>>>>
>>>>>>>> I think this line doesn't affect the build. I removed it and it still
>>>>>>>> compiles (after "make -C samples/bpf clean"). I guess xdp_sample_user.c
>>>>>>>> doesn't include any file in tools/include/uapi. Am I missing something
>>>>>>>> or should I remove this line?
>>>>>>>
>>>>>>> You might have these headers installed on your system already if
>>>>>>> it compiles without this part. So I'd keep this part but do
>>>>>>> s/src/srctree/ (and remove ../..).
>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>                       -I$(LIBBPF_INCLUDE) \
>>>>>>>>>> -                     -I$(src)/../../tools/testing/selftests/bpf
>>>>>>>>>> +                     -I$(srctree)/tools/testing/selftests/bpf
>>>>>>>>>>
>>>>>>>>>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
>>>>>>>>>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
>>>>>>>>>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
>>>>>>>>>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
>>>>>>>>>> $(src)/xdp_sample_shared.h
>>>>>>>>>>          @echo "  CLANG-BPF " $@
>>>>>>>>>>          $(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
>>>>>>>>>> -                -Wno-compare-distinct-pointer-types -I$(srctree)/include \
>>>>>>>>>> +                -Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
>>>>>>>>>>                  -I$(srctree)/samples/bpf -I$(srctree)/tools/include \
>>>>>>>>>>                  -I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
>>>>>>>>>>                  -c $(filter %.bpf.c,$^) -o $@
>>>>>>>>>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
>>>>>>>>>> xdp_sample.bpf.o
>>>>>>>>>>
>>>>>>>>>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
>>>>>>>>>> skel,$(LINKED_SKELS),$($(skel)-deps)))
>>>>>>>>>>
>>>>>>>>>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
>>>>>>>>>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
>>>>>>>>>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
>>>>>>>>>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
>>>>>>>>>>
>>>>>>>>>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
>>>>>>>>>> index 7621f55e2947..86a454cfb080 100644
>>>>>>>>>> --- a/samples/bpf/Makefile.target
>>>>>>>>>> +++ b/samples/bpf/Makefile.target
>>>>>>>>>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
>>>>>>>>>>                   $(TPROGCFLAGS_$(basetarget).o)
>>>>>>>>>>
>>>>>>>>>>  # $(objtree)/$(obj) for including generated headers from checkin source
>>>>>>>>>> files
>>>>>>>>>
>>>>>>>>> [..]
>>>>>>>>>
>>>>>>>>>> -ifeq ($(KBUILD_EXTMOD),)
>>>>>>>>>> +ifneq ($(KBUILD_EXTMOD),)
>>>>>>>>>
>>>>>>>>> This parts seems to be copy-pasted all over the place in its 'ifeq'
>>>>>>>>> form. What is it doing and why is it needed?
>>>>>>>>>
>>>>>>>>>>  ifdef building_out_of_srctree
>>>>>>>>>>  _tprogc_flags   += -I $(objtree)/$(obj)
>>>>>>>>>>  endif
>>>>>>>>>> --
>>>>>>>>>> 2.34.1
>>>>>>
>>>>>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
>>>>>> ---
>>>>>>
>>>>>> Change from the original patch
>>>>>>
>>>>>> - Change "-I$(src)/../../tools/include/uapi" to
>>>>>> "-I$(srctree)/tools/include/uapi"
>>>>>>
>>>>>>  samples/bpf/Makefile        | 10 +++++-----
>>>>>>  samples/bpf/Makefile.target |  2 +-
>>>>>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>>>>>> index 615f24ebc49c..cfc960b3713a 100644
>>>>>> --- a/samples/bpf/Makefile
>>>>>> +++ b/samples/bpf/Makefile
>>>>>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>>>>>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
>>>>>>  # TPROGS_CFLAGS causes conflicts
>>>>>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
>>>>>> -                 -I$(src)/../../tools/include \
>>>>>> -                 -I$(src)/../../tools/include/uapi \
>>>>>> +                 -I$(srctree)/tools/include \
>>>>>> +                 -I$(srctree)/tools/include/uapi \
>>>>>>                   -I$(LIBBPF_INCLUDE) \
>>>>>> -                 -I$(src)/../../tools/testing/selftests/bpf
>>>>>> +                 -I$(srctree)/tools/testing/selftests/bpf
>>>>>>
>>>>>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
>>>>>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
>>>>>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
>>>>>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
>>>>>> $(src)/xdp_sample_shared.h
>>>>>>      @echo "  CLANG-BPF " $@
>>>>>>      $(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
>>>>>> -            -Wno-compare-distinct-pointer-types -I$(srctree)/include \
>>>>>> +            -Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
>>>>>>              -I$(srctree)/samples/bpf -I$(srctree)/tools/include \
>>>>>>              -I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
>>>>>>              -c $(filter %.bpf.c,$^) -o $@
>>>>>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
>>>>>> xdp_sample.bpf.o
>>>>>>
>>>>>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
>>>>>> skel,$(LINKED_SKELS),$($(skel)-deps)))
>>>>>>
>>>>>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
>>>>>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
>>>>>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
>>>>>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
>>>>>>
>>>>>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
>>>>>> index 7621f55e2947..86a454cfb080 100644
>>>>>> --- a/samples/bpf/Makefile.target
>>>>>> +++ b/samples/bpf/Makefile.target
>>>>>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
>>>>>>                   $(TPROGCFLAGS_$(basetarget).o)
>>>>>>
>>>>>>  # $(objtree)/$(obj) for including generated headers from checkin source
>>>>>> files
>>>>>
>>>>> [..]
>>>>>
>>>>>> -ifeq ($(KBUILD_EXTMOD),)
>>>>>> +ifneq ($(KBUILD_EXTMOD),)
>>>>>>  ifdef building_out_of_srctree
>>>>>>  _tprogc_flags   += -I $(objtree)/$(obj)
>>>>>>  endif
>>>>>
>>>>> This question left undressed. Can you share more on why this change
>>>>> is needed? Because it looks like it's actually needed for M='' case.
>>>>> IOW, maybe we should add $(objtree)/$(obj) somewhere else?
>>>>
>>>> If it's needed for both cases M='' and M != '', is it better to just
>>>> remove the condition with $(KBUILD_EXTMOD)? FMPOV, -I $(objtree)/$(obj)
>>>> is only needed in case of building_out_of_srctree, no matter what
>>>> KBUILD_EXTMOD.
>>>
>>> My guess is that we're missing -I $(objtree)/$(obj) somewhere else.
>>> Depending on what/where it fails, we probably need to add $(objtree)
>>> to some of the $(obj)s?
>>> I'm mostly speculating here, because I see that "ifeq
>>> ($(KBUILD_EXTMOD),) + ifdef building_out_of_srctree" pattern being
>>> used in the other places.
>>
>> It turns out that your guess is right. "-I $(objtree)/$(obj)" is only
>> needed for the following objects: xdp_redirect_map_multi_user,
>> xdp_redirect_cpu_user, xdp_redirect_map_user, xdp_redirect_user,
>> xdp_monitor_user and xdp_router_ipv4_user. There is a variable to add
>> flags for gcc named $(TPROGCFLAGS_$(basetarget).o) but it has not been
>> set. My proposed fix is to set the following variables:
> 
> Looks like these are the tests that use bpf skeleton.
> 
>> +TPROGCFLAGS_xdp_redirect_map_multi_user.o += -I$(objtree)/$(obj)
>> +TPROGCFLAGS_xdp_redirect_cpu_user.o += -I$(objtree)/$(obj)
>> +TPROGCFLAGS_xdp_redirect_map_user.o += -I$(objtree)/$(obj)
>> +TPROGCFLAGS_xdp_redirect_user.o += -I$(objtree)/$(obj)
>> +TPROGCFLAGS_xdp_monitor_user.o += -I$(objtree)/$(obj)
>> +TPROGCFLAGS_xdp_router_ipv4_user.o += -I$(objtree)/$(obj)
>>
>> It works with the original "ifeq
>>> ($(KBUILD_EXTMOD),) + ifdef building_out_of_srctree" pattern. What do
>> you think about my proposed fix?
> 
> Maybe we should just unconditionally add "-I $(objtree)/$(obj)" to
> _tprogc_flags ?
> 
> And then we can drop that whole part:
> ifeq ($(KBUILD_EXTMOD),)
> ifdef building_out_of_srctree
> _tprogc_flags   += -I $(objtree)/$(obj)
> endif
> endif
> 
> IOW, replace that $(TPROGCFLAGS_$(basetarget).o) part with -I
> $(objtree)/$(obj) and call it a day?
> I honestly have no clue why it's this complicated and I doubt anybody
> at this point remembers...

That is the simplest way. I intended to do it but I didn't as I think
the condition is there for some reasons that I haven't understood yet.


^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v1] samples/bpf: Fix build out of source tree
@ 2023-07-21  1:05                     ` Anh Tuan Phan
  0 siblings, 0 replies; 24+ messages in thread
From: Anh Tuan Phan @ 2023-07-21  1:05 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: daniel, martin.lau, ast, andrii, bpf, linux-kernel-mentees



On 7/21/23 00:05, Stanislav Fomichev wrote:
> On Wed, Jul 19, 2023 at 9:13 AM Anh Tuan Phan <tuananhlfc@gmail.com> wrote:
>>
>>
>>
>> On 7/17/23 23:46, Stanislav Fomichev wrote:
>>> On Sun, Jul 16, 2023 at 2:42 AM Anh Tuan Phan <tuananhlfc@gmail.com> wrote:
>>>>
>>>>
>>>>
>>>> On 7/11/23 00:18, Stanislav Fomichev wrote:
>>>>> On 07/09, Anh Tuan Phan wrote:
>>>>>> I updated the patch to reflect your suggestion. Thank you!
>>>>>
>>>>> In the future, can you please post a new one with v+1 instead of replying
>>>>> to the old one? Thx!
>>>>
>>>> Will do in the next version.
>>>>
>>>>>
>>>>>> On 7/7/23 23:53, Stanislav Fomichev wrote:
>>>>>>> On 07/07, Anh Tuan Phan wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 7/7/23 01:09, Stanislav Fomichev wrote:
>>>>>>>>> On 07/06, Anh Tuan Phan wrote:
>>>>>>>>>> This commit fixes a few compilation issues when building out of source
>>>>>>>>>> tree. The command that I used to build samples/bpf:
>>>>>>>>>>
>>>>>>>>>> export KBUILD_OUTPUT=/tmp
>>>>>>>>>> make V=1 M=samples/bpf
>>>>>>>>>>
>>>>>>>>>> The compilation failed since it tried to find the header files in the
>>>>>>>>>> wrong places between output directory and source tree directory
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
>>>>>>>>>> ---
>>>>>>>>>>  samples/bpf/Makefile        | 8 ++++----
>>>>>>>>>>  samples/bpf/Makefile.target | 2 +-
>>>>>>>>>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>>>>>>>>>> index 615f24ebc49c..32469aaa82d5 100644
>>>>>>>>>> --- a/samples/bpf/Makefile
>>>>>>>>>> +++ b/samples/bpf/Makefile
>>>>>>>>>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>>>>>>>>>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
>>>>>>>>>>  # TPROGS_CFLAGS causes conflicts
>>>>>>>>>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
>>>>>>>>>> -                     -I$(src)/../../tools/include \
>>>>>>>>>> +                     -I$(srctree)/tools/include \
>>>>>>>>>
>>>>>>>>> [..]
>>>>>>>>>
>>>>>>>>>>                       -I$(src)/../../tools/include/uapi \
>>>>>>>>>
>>>>>>>>> Does this $(src) need to be changed as well?
>>>>>>>>
>>>>>>>> I think this line doesn't affect the build. I removed it and it still
>>>>>>>> compiles (after "make -C samples/bpf clean"). I guess xdp_sample_user.c
>>>>>>>> doesn't include any file in tools/include/uapi. Am I missing something
>>>>>>>> or should I remove this line?
>>>>>>>
>>>>>>> You might have these headers installed on your system already if
>>>>>>> it compiles without this part. So I'd keep this part but do
>>>>>>> s/src/srctree/ (and remove ../..).
>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>                       -I$(LIBBPF_INCLUDE) \
>>>>>>>>>> -                     -I$(src)/../../tools/testing/selftests/bpf
>>>>>>>>>> +                     -I$(srctree)/tools/testing/selftests/bpf
>>>>>>>>>>
>>>>>>>>>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
>>>>>>>>>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
>>>>>>>>>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
>>>>>>>>>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
>>>>>>>>>> $(src)/xdp_sample_shared.h
>>>>>>>>>>          @echo "  CLANG-BPF " $@
>>>>>>>>>>          $(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
>>>>>>>>>> -                -Wno-compare-distinct-pointer-types -I$(srctree)/include \
>>>>>>>>>> +                -Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
>>>>>>>>>>                  -I$(srctree)/samples/bpf -I$(srctree)/tools/include \
>>>>>>>>>>                  -I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
>>>>>>>>>>                  -c $(filter %.bpf.c,$^) -o $@
>>>>>>>>>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
>>>>>>>>>> xdp_sample.bpf.o
>>>>>>>>>>
>>>>>>>>>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
>>>>>>>>>> skel,$(LINKED_SKELS),$($(skel)-deps)))
>>>>>>>>>>
>>>>>>>>>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
>>>>>>>>>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
>>>>>>>>>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
>>>>>>>>>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
>>>>>>>>>>
>>>>>>>>>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
>>>>>>>>>> index 7621f55e2947..86a454cfb080 100644
>>>>>>>>>> --- a/samples/bpf/Makefile.target
>>>>>>>>>> +++ b/samples/bpf/Makefile.target
>>>>>>>>>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
>>>>>>>>>>                   $(TPROGCFLAGS_$(basetarget).o)
>>>>>>>>>>
>>>>>>>>>>  # $(objtree)/$(obj) for including generated headers from checkin source
>>>>>>>>>> files
>>>>>>>>>
>>>>>>>>> [..]
>>>>>>>>>
>>>>>>>>>> -ifeq ($(KBUILD_EXTMOD),)
>>>>>>>>>> +ifneq ($(KBUILD_EXTMOD),)
>>>>>>>>>
>>>>>>>>> This parts seems to be copy-pasted all over the place in its 'ifeq'
>>>>>>>>> form. What is it doing and why is it needed?
>>>>>>>>>
>>>>>>>>>>  ifdef building_out_of_srctree
>>>>>>>>>>  _tprogc_flags   += -I $(objtree)/$(obj)
>>>>>>>>>>  endif
>>>>>>>>>> --
>>>>>>>>>> 2.34.1
>>>>>>
>>>>>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
>>>>>> ---
>>>>>>
>>>>>> Change from the original patch
>>>>>>
>>>>>> - Change "-I$(src)/../../tools/include/uapi" to
>>>>>> "-I$(srctree)/tools/include/uapi"
>>>>>>
>>>>>>  samples/bpf/Makefile        | 10 +++++-----
>>>>>>  samples/bpf/Makefile.target |  2 +-
>>>>>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>>>>>> index 615f24ebc49c..cfc960b3713a 100644
>>>>>> --- a/samples/bpf/Makefile
>>>>>> +++ b/samples/bpf/Makefile
>>>>>> @@ -341,10 +341,10 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>>>>>>  # Override includes for xdp_sample_user.o because $(srctree)/usr/include in
>>>>>>  # TPROGS_CFLAGS causes conflicts
>>>>>>  XDP_SAMPLE_CFLAGS += -Wall -O2 \
>>>>>> -                 -I$(src)/../../tools/include \
>>>>>> -                 -I$(src)/../../tools/include/uapi \
>>>>>> +                 -I$(srctree)/tools/include \
>>>>>> +                 -I$(srctree)/tools/include/uapi \
>>>>>>                   -I$(LIBBPF_INCLUDE) \
>>>>>> -                 -I$(src)/../../tools/testing/selftests/bpf
>>>>>> +                 -I$(srctree)/tools/testing/selftests/bpf
>>>>>>
>>>>>>  $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS)
>>>>>>  $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h
>>>>>> @@ -393,7 +393,7 @@ $(obj)/xdp_router_ipv4.bpf.o: $(obj)/xdp_sample.bpf.o
>>>>>>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h
>>>>>> $(src)/xdp_sample_shared.h
>>>>>>      @echo "  CLANG-BPF " $@
>>>>>>      $(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
>>>>>> -            -Wno-compare-distinct-pointer-types -I$(srctree)/include \
>>>>>> +            -Wno-compare-distinct-pointer-types -I$(obj) -I$(srctree)/include \
>>>>>>              -I$(srctree)/samples/bpf -I$(srctree)/tools/include \
>>>>>>              -I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
>>>>>>              -c $(filter %.bpf.c,$^) -o $@
>>>>>> @@ -412,7 +412,7 @@ xdp_router_ipv4.skel.h-deps := xdp_router_ipv4.bpf.o
>>>>>> xdp_sample.bpf.o
>>>>>>
>>>>>>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach
>>>>>> skel,$(LINKED_SKELS),$($(skel)-deps)))
>>>>>>
>>>>>> -BPF_SRCS_LINKED := $(notdir $(wildcard $(src)/*.bpf.c))
>>>>>> +BPF_SRCS_LINKED := $(notdir $(wildcard $(srctree)/$(src)/*.bpf.c))
>>>>>>  BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(BPF_SRCS_LINKED))
>>>>>>  BPF_SKELS_LINKED := $(addprefix $(obj)/,$(LINKED_SKELS))
>>>>>>
>>>>>> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target
>>>>>> index 7621f55e2947..86a454cfb080 100644
>>>>>> --- a/samples/bpf/Makefile.target
>>>>>> +++ b/samples/bpf/Makefile.target
>>>>>> @@ -41,7 +41,7 @@ _tprogc_flags   = $(TPROGS_CFLAGS) \
>>>>>>                   $(TPROGCFLAGS_$(basetarget).o)
>>>>>>
>>>>>>  # $(objtree)/$(obj) for including generated headers from checkin source
>>>>>> files
>>>>>
>>>>> [..]
>>>>>
>>>>>> -ifeq ($(KBUILD_EXTMOD),)
>>>>>> +ifneq ($(KBUILD_EXTMOD),)
>>>>>>  ifdef building_out_of_srctree
>>>>>>  _tprogc_flags   += -I $(objtree)/$(obj)
>>>>>>  endif
>>>>>
>>>>> This question left undressed. Can you share more on why this change
>>>>> is needed? Because it looks like it's actually needed for M='' case.
>>>>> IOW, maybe we should add $(objtree)/$(obj) somewhere else?
>>>>
>>>> If it's needed for both cases M='' and M != '', is it better to just
>>>> remove the condition with $(KBUILD_EXTMOD)? FMPOV, -I $(objtree)/$(obj)
>>>> is only needed in case of building_out_of_srctree, no matter what
>>>> KBUILD_EXTMOD.
>>>
>>> My guess is that we're missing -I $(objtree)/$(obj) somewhere else.
>>> Depending on what/where it fails, we probably need to add $(objtree)
>>> to some of the $(obj)s?
>>> I'm mostly speculating here, because I see that "ifeq
>>> ($(KBUILD_EXTMOD),) + ifdef building_out_of_srctree" pattern being
>>> used in the other places.
>>
>> It turns out that your guess is right. "-I $(objtree)/$(obj)" is only
>> needed for the following objects: xdp_redirect_map_multi_user,
>> xdp_redirect_cpu_user, xdp_redirect_map_user, xdp_redirect_user,
>> xdp_monitor_user and xdp_router_ipv4_user. There is a variable to add
>> flags for gcc named $(TPROGCFLAGS_$(basetarget).o) but it has not been
>> set. My proposed fix is to set the following variables:
> 
> Looks like these are the tests that use bpf skeleton.
> 
>> +TPROGCFLAGS_xdp_redirect_map_multi_user.o += -I$(objtree)/$(obj)
>> +TPROGCFLAGS_xdp_redirect_cpu_user.o += -I$(objtree)/$(obj)
>> +TPROGCFLAGS_xdp_redirect_map_user.o += -I$(objtree)/$(obj)
>> +TPROGCFLAGS_xdp_redirect_user.o += -I$(objtree)/$(obj)
>> +TPROGCFLAGS_xdp_monitor_user.o += -I$(objtree)/$(obj)
>> +TPROGCFLAGS_xdp_router_ipv4_user.o += -I$(objtree)/$(obj)
>>
>> It works with the original "ifeq
>>> ($(KBUILD_EXTMOD),) + ifdef building_out_of_srctree" pattern. What do
>> you think about my proposed fix?
> 
> Maybe we should just unconditionally add "-I $(objtree)/$(obj)" to
> _tprogc_flags ?
> 
> And then we can drop that whole part:
> ifeq ($(KBUILD_EXTMOD),)
> ifdef building_out_of_srctree
> _tprogc_flags   += -I $(objtree)/$(obj)
> endif
> endif
> 
> IOW, replace that $(TPROGCFLAGS_$(basetarget).o) part with -I
> $(objtree)/$(obj) and call it a day?
> I honestly have no clue why it's this complicated and I doubt anybody
> at this point remembers...

That is the simplest way. I intended to do it but I didn't as I think
the condition is there for some reasons that I haven't understood yet.

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 24+ messages in thread
end of thread, other threads:[~2023-07-21  1:05 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-06 15:25 [PATCH v1] samples/bpf: Fix build out of source tree Anh Tuan Phan
2023-07-06 15:25 ` Anh Tuan Phan
2023-07-06 18:09 ` Stanislav Fomichev
2023-07-06 18:09   ` Stanislav Fomichev via Linux-kernel-mentees
2023-07-07  1:10   ` Anh Tuan Phan
2023-07-07  1:10     ` Anh Tuan Phan
2023-07-07 16:53     ` Stanislav Fomichev
2023-07-07 16:53       ` Stanislav Fomichev via Linux-kernel-mentees
2023-07-09 14:45       ` Anh Tuan Phan
2023-07-09 14:45         ` Anh Tuan Phan
2023-07-10 17:18         ` Stanislav Fomichev
2023-07-10 17:18           ` Stanislav Fomichev via Linux-kernel-mentees
2023-07-16  9:42           ` Anh Tuan Phan
2023-07-16  9:42             ` Anh Tuan Phan
2023-07-17 16:46             ` Stanislav Fomichev
2023-07-17 16:46               ` Stanislav Fomichev via Linux-kernel-mentees
2023-07-19 16:13               ` Anh Tuan Phan
2023-07-19 16:13                 ` Anh Tuan Phan
2023-07-20 17:05                 ` Stanislav Fomichev
2023-07-20 17:05                   ` Stanislav Fomichev via Linux-kernel-mentees
2023-07-21  1:05                   ` Anh Tuan Phan
2023-07-21  1:05                     ` Anh Tuan Phan
2023-07-09  5:12   ` Anh Tuan Phan
2023-07-09  5:12     ` Anh Tuan Phan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.