On Sun, Jul 16, 2023 at 2:42 AM Anh Tuan Phan <tuananhlfc@xxxxxxxxx> 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@xxxxxxxxx> > >>>>>> --- > >>>>>> 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@xxxxxxxxx> > >> --- > >> > >> 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.
