Re: [net-next PATCH] samples/bpf: adjust Makefile and README.rst

Re: [net-next PATCH] samples/bpf: adjust Makefile and README.rst

[Daniel Borkmann]

On 11/10/19 5:31 PM, Jesper Dangaard Brouer wrote:
> Side effect of some kbuild changes resulted in breaking the
> documented way to build samples/bpf/.
> 
> This patch change the samples/bpf/Makefile to work again, when
> invoking make from the subdir samples/bpf/. Also update the
> documentation in README.rst, to reflect the new way to build.
> 
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>

Please make sure to have bpf@vger.kernel.org Cc'ed in future as well
(done here). Given net-next in subject, any specific reason you need
this expedited over normal bpf-next route? Looks like there is no
conflict either way.

In any case, change looks good to me:

Acked-by: Daniel Borkmann <daniel@iogearbox.net>

> ---
>   samples/bpf/Makefile   |    4 ++--
>   samples/bpf/README.rst |   12 +++++-------
>   2 files changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index d88c01275239..8e32a4d29a38 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -203,7 +203,7 @@ TPROGLDLIBS_test_overhead	+= -lrt
>   TPROGLDLIBS_xdpsock		+= -pthread
>   
>   # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline:
> -#  make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
> +#  make M=samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
>   LLC ?= llc
>   CLANG ?= clang
>   LLVM_OBJCOPY ?= llvm-objcopy
> @@ -246,7 +246,7 @@ endif
>   
>   # Trick to allow make to be run from this directory
>   all:
> -	$(MAKE) -C ../../ $(CURDIR)/ BPF_SAMPLES_PATH=$(CURDIR)
> +	$(MAKE) -C ../../ M=$(CURDIR) BPF_SAMPLES_PATH=$(CURDIR)
>   
>   clean:
>   	$(MAKE) -C ../../ M=$(CURDIR) clean
> diff --git a/samples/bpf/README.rst b/samples/bpf/README.rst
> index cc1f00a1ee06..dd34b2d26f1c 100644
> --- a/samples/bpf/README.rst
> +++ b/samples/bpf/README.rst
> @@ -46,12 +46,10 @@ Compiling
>   For building the BPF samples, issue the below command from the kernel
>   top level directory::
>   
> - make samples/bpf/
> -
> -Do notice the "/" slash after the directory name.
> + make M=samples/bpf
>   
>   It is also possible to call make from this directory.  This will just
> -hide the the invocation of make as above with the appended "/".
> +hide the invocation of make as above.
>   
>   Manually compiling LLVM with 'bpf' support
>   ------------------------------------------
> @@ -77,7 +75,7 @@ Quick sniplet for manually compiling LLVM and clang
>   It is also possible to point make to the newly compiled 'llc' or
>   'clang' command via redefining LLC or CLANG on the make command line::
>   
> - make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
> + make M=samples/bpf LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
>   
>   Cross compiling samples
>   -----------------------
> @@ -98,10 +96,10 @@ Pointing LLC and CLANG is not necessarily if it's installed on HOST and have
>   in its targets appropriate arm64 arch (usually it has several arches).
>   Build samples::
>   
> - make samples/bpf/
> + make M=samples/bpf
>   
>   Or build samples with SYSROOT if some header or library is absent in toolchain,
>   say libelf, providing address to file system containing headers and libs,
>   can be RFS of target board::
>   
> - make samples/bpf/ SYSROOT=~/some_sysroot
> + make M=samples/bpf SYSROOT=~/some_sysroot
> 

Re: [net-next PATCH] samples/bpf: adjust Makefile and README.rst

[Jesper Dangaard Brouer]

On Mon, 11 Nov 2019 14:49:51 +0100
Daniel Borkmann <daniel@iogearbox.net> wrote:

> On 11/10/19 5:31 PM, Jesper Dangaard Brouer wrote:
> > Side effect of some kbuild changes resulted in breaking the
> > documented way to build samples/bpf/.
> > 
> > This patch change the samples/bpf/Makefile to work again, when
> > invoking make from the subdir samples/bpf/. Also update the
> > documentation in README.rst, to reflect the new way to build.
> > 
> > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>  
> 
> Please make sure to have bpf@vger.kernel.org Cc'ed in future as well
> (done here). Given net-next in subject, any specific reason you need
> this expedited over normal bpf-next route? Looks like there is no
> conflict either way.

When I created this patch, bpf-next didn't have the other fixes for
samples/bpf/.  If you have sync'ed with net-next, then I'm fine with
you taking this change (as it will propagate back to DaveM's tree soon
enough).


> In any case, change looks good to me:
> 
> Acked-by: Daniel Borkmann <daniel@iogearbox.net>
> 
> > ---
> >   samples/bpf/Makefile   |    4 ++--
> >   samples/bpf/README.rst |   12 +++++-------
> >   2 files changed, 7 insertions(+), 9 deletions(-)
> > 
> > diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> > index d88c01275239..8e32a4d29a38 100644
> > --- a/samples/bpf/Makefile
> > +++ b/samples/bpf/Makefile
> > @@ -203,7 +203,7 @@ TPROGLDLIBS_test_overhead	+= -lrt
> >   TPROGLDLIBS_xdpsock		+= -pthread
> >   
> >   # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline:
> > -#  make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
> > +#  make M=samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
> >   LLC ?= llc
> >   CLANG ?= clang
> >   LLVM_OBJCOPY ?= llvm-objcopy
> > @@ -246,7 +246,7 @@ endif
> >   
> >   # Trick to allow make to be run from this directory
> >   all:
> > -	$(MAKE) -C ../../ $(CURDIR)/ BPF_SAMPLES_PATH=$(CURDIR)
> > +	$(MAKE) -C ../../ M=$(CURDIR) BPF_SAMPLES_PATH=$(CURDIR)
> >   
> >   clean:
> >   	$(MAKE) -C ../../ M=$(CURDIR) clean
> > diff --git a/samples/bpf/README.rst b/samples/bpf/README.rst
> > index cc1f00a1ee06..dd34b2d26f1c 100644
> > --- a/samples/bpf/README.rst
> > +++ b/samples/bpf/README.rst
> > @@ -46,12 +46,10 @@ Compiling
> >   For building the BPF samples, issue the below command from the kernel
> >   top level directory::
> >   
> > - make samples/bpf/
> > -
> > -Do notice the "/" slash after the directory name.
> > + make M=samples/bpf
> >   
> >   It is also possible to call make from this directory.  This will just
> > -hide the the invocation of make as above with the appended "/".
> > +hide the invocation of make as above.
> >   
> >   Manually compiling LLVM with 'bpf' support
> >   ------------------------------------------
> > @@ -77,7 +75,7 @@ Quick sniplet for manually compiling LLVM and clang
> >   It is also possible to point make to the newly compiled 'llc' or
> >   'clang' command via redefining LLC or CLANG on the make command line::
> >   
> > - make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
> > + make M=samples/bpf LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
> >   
> >   Cross compiling samples
> >   -----------------------
> > @@ -98,10 +96,10 @@ Pointing LLC and CLANG is not necessarily if it's installed on HOST and have
> >   in its targets appropriate arm64 arch (usually it has several arches).
> >   Build samples::
> >   
> > - make samples/bpf/
> > + make M=samples/bpf
> >   
> >   Or build samples with SYSROOT if some header or library is absent in toolchain,
> >   say libelf, providing address to file system containing headers and libs,
> >   can be RFS of target board::
> >   
> > - make samples/bpf/ SYSROOT=~/some_sysroot
> > + make M=samples/bpf SYSROOT=~/some_sysroot
> >   
> 



-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

Re: [net-next PATCH] samples/bpf: adjust Makefile and README.rst

[Daniel Borkmann]

On 11/11/19 2:56 PM, Jesper Dangaard Brouer wrote:
> On Mon, 11 Nov 2019 14:49:51 +0100
> Daniel Borkmann <daniel@iogearbox.net> wrote:
>> On 11/10/19 5:31 PM, Jesper Dangaard Brouer wrote:
>>> Side effect of some kbuild changes resulted in breaking the
>>> documented way to build samples/bpf/.
>>>
>>> This patch change the samples/bpf/Makefile to work again, when
>>> invoking make from the subdir samples/bpf/. Also update the
>>> documentation in README.rst, to reflect the new way to build.
>>>
>>> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
>>
>> Please make sure to have bpf@vger.kernel.org Cc'ed in future as well
>> (done here). Given net-next in subject, any specific reason you need
>> this expedited over normal bpf-next route? Looks like there is no
>> conflict either way.
> 
> When I created this patch, bpf-next didn't have the other fixes for
> samples/bpf/.  If you have sync'ed with net-next, then I'm fine with

Correct, as they were just merged by Linus over the weekend. For bpf-next,
we fast-fwd after every PR that is sent for net-next. For fixes the route
is bpf -> net -> linus -> net-next -> bpf-next.

> you taking this change (as it will propagate back to DaveM's tree soon
> enough).

Ok, I don't think the M= adjustment to get it back in line with the samples
README is urgent, but lets go with net-next directly then and have it
propagate back later this week, sent my Acked-by previously for carrying on.

Thanks,
Daniel