Linux Kernel Mentees list
 help / color / mirror / Atom feed
From: Stanislav Fomichev via Linux-kernel-mentees <linux-kernel-mentees@lists.linuxfoundation.org>
To: Anh Tuan Phan <tuananhlfc@gmail.com>
Cc: daniel@iogearbox.net, martin.lau@linux.dev, ast@kernel.org,
	andrii@kernel.org, bpf@vger.kernel.org,
	linux-kernel-mentees@lists.linuxfoundation.org
Subject: Re: [PATCH v1] samples/bpf: Fix build out of source tree
Date: Mon, 10 Jul 2023 10:18:21 -0700	[thread overview]
Message-ID: <ZKw9XQGOza6qGDLf@google.com> (raw)
In-Reply-To: <2b48be65-5f63-4658-38cb-03c00c10fdf3@gmail.com>

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

  reply	other threads:[~2023-07-10 17:18 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-06 15:25 [PATCH v1] samples/bpf: Fix build out of source tree Anh Tuan Phan
2023-07-06 18:09 ` Stanislav Fomichev via Linux-kernel-mentees
2023-07-07  1:10   ` Anh Tuan Phan
2023-07-07 16:53     ` Stanislav Fomichev via Linux-kernel-mentees
2023-07-09 14:45       ` Anh Tuan Phan
2023-07-10 17:18         ` Stanislav Fomichev via Linux-kernel-mentees [this message]
2023-07-16  9:42           ` Anh Tuan Phan
2023-07-17 16:46             ` Stanislav Fomichev via Linux-kernel-mentees
2023-07-19 16:13               ` Anh Tuan Phan
2023-07-20 17:05                 ` Stanislav Fomichev via Linux-kernel-mentees
2023-07-21  1:05                   ` Anh Tuan Phan
2023-07-09  5:12   ` Anh Tuan Phan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZKw9XQGOza6qGDLf@google.com \
    --to=linux-kernel-mentees@lists.linuxfoundation.org \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=martin.lau@linux.dev \
    --cc=sdf@google.com \
    --cc=tuananhlfc@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox