mbox series

[0/8] selftests/bpf: installation and out of tree build fixes

Message ID 20200522041310.233185-1-yauheni.kaliuta@redhat.com
Headers show
Series selftests/bpf: installation and out of tree build fixes | expand

Message

Yauheni Kaliuta May 22, 2020, 4:13 a.m. UTC
I had a look, here are some fixes.

Yauheni Kaliuta (8):
  selftests/bpf: remove test_align from Makefile
  selftests/bpf: build bench.o for any $(OUTPUT)
  selftests/bpf: install btf .c files
  selftests/bpf: fix object files installation
  selftests/bpf: add output dir to include list
  selftests/bpf: fix urandom_read installation
  selftests/bpf: fix test.h placing for out of tree build
  selftests/bpf: factor out MKDIR rule

 tools/testing/selftests/bpf/Makefile | 77 ++++++++++++++++++++--------
 1 file changed, 55 insertions(+), 22 deletions(-)

Comments

Yauheni Kaliuta May 22, 2020, 6:40 a.m. UTC | #1
Actually, a bit more needed :)

>>>>> On Fri, 22 May 2020 07:13:02 +0300, Yauheni Kaliuta  wrote:

 > I had a look, here are some fixes.
 > Yauheni Kaliuta (8):
 >   selftests/bpf: remove test_align from Makefile
 >   selftests/bpf: build bench.o for any $(OUTPUT)
 >   selftests/bpf: install btf .c files
 >   selftests/bpf: fix object files installation
 >   selftests/bpf: add output dir to include list
 >   selftests/bpf: fix urandom_read installation
 >   selftests/bpf: fix test.h placing for out of tree build
 >   selftests/bpf: factor out MKDIR rule

 >  tools/testing/selftests/bpf/Makefile | 77 ++++++++++++++++++++--------
 >  1 file changed, 55 insertions(+), 22 deletions(-)

 > -- 
 > 2.26.2
Daniel Borkmann May 26, 2020, 9:48 p.m. UTC | #2
On 5/22/20 8:40 AM, Yauheni Kaliuta wrote:
> 
> Actually, a bit more needed :)

Not quite sure how to parse this, I presume you are intending to send a v2 of
this series with [0] folded in? Please also do not add line-breaks in the middle
of all your Fixes tags as otherwise it would break searching for commits in the
git log. For the v2 respin, please also add a better cover letter than just saying
nothing more than 'I had a look, here are some fixes.'. At least a minimal high
level summary of the selftest Makefile changes in this series.

Thanks,
Daniel

   [0] https://patchwork.ozlabs.org/project/netdev/patch/20200522081901.238516-1-yauheni.kaliuta@redhat.com/

>>>>>> On Fri, 22 May 2020 07:13:02 +0300, Yauheni Kaliuta  wrote:
> 
>   > I had a look, here are some fixes.
>   > Yauheni Kaliuta (8):
>   >   selftests/bpf: remove test_align from Makefile
>   >   selftests/bpf: build bench.o for any $(OUTPUT)
>   >   selftests/bpf: install btf .c files
>   >   selftests/bpf: fix object files installation
>   >   selftests/bpf: add output dir to include list
>   >   selftests/bpf: fix urandom_read installation
>   >   selftests/bpf: fix test.h placing for out of tree build
>   >   selftests/bpf: factor out MKDIR rule
> 
>   >  tools/testing/selftests/bpf/Makefile | 77 ++++++++++++++++++++--------
>   >  1 file changed, 55 insertions(+), 22 deletions(-)
> 
>   > --
>   > 2.26.2
> 
>
Andrii Nakryiko May 26, 2020, 10:32 p.m. UTC | #3
On Thu, May 21, 2020 at 11:41 PM Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
>
>
> Actually, a bit more needed :)

From the other kselftest thread, it seems like selftests are not
supporting builds out-of-tree. With that, wouldn't it be simpler to
build in tree and then just copy selftests/bpf directory to wherever
you need to run tests from? It would be simple and reliable. Given I
and probably everyone else never build and run tests out-of-tree, it's
just too easy to break this and you'll be constantly chasing some
non-obvious breakages...

Is there some problem with such approach?

>
> >>>>> On Fri, 22 May 2020 07:13:02 +0300, Yauheni Kaliuta  wrote:
>
>  > I had a look, here are some fixes.
>  > Yauheni Kaliuta (8):
>  >   selftests/bpf: remove test_align from Makefile
>  >   selftests/bpf: build bench.o for any $(OUTPUT)
>  >   selftests/bpf: install btf .c files
>  >   selftests/bpf: fix object files installation
>  >   selftests/bpf: add output dir to include list
>  >   selftests/bpf: fix urandom_read installation
>  >   selftests/bpf: fix test.h placing for out of tree build
>  >   selftests/bpf: factor out MKDIR rule
>
>  >  tools/testing/selftests/bpf/Makefile | 77 ++++++++++++++++++++--------
>  >  1 file changed, 55 insertions(+), 22 deletions(-)
>
>  > --
>  > 2.26.2
>
>
> --
> WBR,
> Yauheni Kaliuta
>
Yauheni Kaliuta May 27, 2020, 4:45 a.m. UTC | #4
Hi, Daniel!

>>>>> On Tue, 26 May 2020 23:48:01 +0200, Daniel Borkmann  wrote:

 > On 5/22/20 8:40 AM, Yauheni Kaliuta wrote:
 >> 
 >> Actually, a bit more needed :)

 > Not quite sure how to parse this, I presume you are intending
 > to send a v2 of this series with [0] folded in? Please also do
 > not add line-breaks in the middle of all your Fixes tags as
 > otherwise it would break searching for commits in the git
 > log. For the v2 respin, please also add a better cover letter
 > than just saying nothing more than 'I had a look, here are
 > some fixes.'. At least a minimal high level summary of the
 > selftest Makefile changes in this series.

Thanks! That was part of thread with Andrii, but I should have
sent separated. Anyway (see Andrii's comments) it's not coming as
is, so I'll do v2. Sorry for that. 

 > Thanks,
 > Daniel

 >   [0]
 > https://patchwork.ozlabs.org/project/netdev/patch/20200522081901.238516-1-yauheni.kaliuta@redhat.com/

 >>>>>>> On Fri, 22 May 2020 07:13:02 +0300, Yauheni Kaliuta  wrote:
 >> 
 >> > I had a look, here are some fixes.
 >> > Yauheni Kaliuta (8):
 >> >   selftests/bpf: remove test_align from Makefile
 >> >   selftests/bpf: build bench.o for any $(OUTPUT)
 >> >   selftests/bpf: install btf .c files
 >> >   selftests/bpf: fix object files installation
 >> >   selftests/bpf: add output dir to include list
 >> >   selftests/bpf: fix urandom_read installation
 >> >   selftests/bpf: fix test.h placing for out of tree build
 >> >   selftests/bpf: factor out MKDIR rule
 >> 
 >> >  tools/testing/selftests/bpf/Makefile | 77 ++++++++++++++++++++--------
 >> >  1 file changed, 55 insertions(+), 22 deletions(-)
 >> 
 >> > --
 >> > 2.26.2
 >> 
 >>
Yauheni Kaliuta May 27, 2020, 4:52 a.m. UTC | #5
Hi, Andrii!

>>>>> On Tue, 26 May 2020 15:32:10 -0700, Andrii Nakryiko  wrote:

 > On Thu, May 21, 2020 at 11:41 PM Yauheni Kaliuta
 > <yauheni.kaliuta@redhat.com> wrote:
 >> 
 >> 
 >> Actually, a bit more needed :)

 > From the other kselftest thread, it seems like selftests are not
 > supporting builds out-of-tree. With that, wouldn't it be simpler to
 > build in tree and then just copy selftests/bpf directory to wherever
 > you need to run tests from? It would be simple and reliable. Given I
 > and probably everyone else never build and run tests out-of-tree, it's
 > just too easy to break this and you'll be constantly chasing some
 > non-obvious breakages...

 > Is there some problem with such approach?

This is `make install` ;).

I personally do not need OOT build, but since it's in the code,
I'd prefer either fix it or remove it, otherwise it's
misleading. But I have not got reply from kselftest.

 >> 
 >> >>>>> On Fri, 22 May 2020 07:13:02 +0300, Yauheni Kaliuta  wrote:
 >> 
 >> > I had a look, here are some fixes.
 >> > Yauheni Kaliuta (8):
 >> >   selftests/bpf: remove test_align from Makefile
 >> >   selftests/bpf: build bench.o for any $(OUTPUT)
 >> >   selftests/bpf: install btf .c files
 >> >   selftests/bpf: fix object files installation
 >> >   selftests/bpf: add output dir to include list
 >> >   selftests/bpf: fix urandom_read installation
 >> >   selftests/bpf: fix test.h placing for out of tree build
 >> >   selftests/bpf: factor out MKDIR rule
 >> 
 >> >  tools/testing/selftests/bpf/Makefile | 77 ++++++++++++++++++++--------
 >> >  1 file changed, 55 insertions(+), 22 deletions(-)
 >> 
 >> > --
 >> > 2.26.2
 >> 
 >> 
 >> --
 >> WBR,
 >> Yauheni Kaliuta
 >>
Andrii Nakryiko May 27, 2020, 5:04 a.m. UTC | #6
On Tue, May 26, 2020 at 9:52 PM Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
>
> Hi, Andrii!
>
> >>>>> On Tue, 26 May 2020 15:32:10 -0700, Andrii Nakryiko  wrote:
>
>  > On Thu, May 21, 2020 at 11:41 PM Yauheni Kaliuta
>  > <yauheni.kaliuta@redhat.com> wrote:
>  >>
>  >>
>  >> Actually, a bit more needed :)
>
>  > From the other kselftest thread, it seems like selftests are not
>  > supporting builds out-of-tree. With that, wouldn't it be simpler to
>  > build in tree and then just copy selftests/bpf directory to wherever
>  > you need to run tests from? It would be simple and reliable. Given I
>  > and probably everyone else never build and run tests out-of-tree, it's
>  > just too easy to break this and you'll be constantly chasing some
>  > non-obvious breakages...
>
>  > Is there some problem with such approach?
>
> This is `make install` ;).

So patch #2, #5, and #7 is solving just `make install` problem?..

My point is that by building in tree and then just copying everything
under selftests/bpf directory to wherever you want to "install" it
would just work. And won't require complicating already complicated
Makefile. Any problem with such approach?


>
> I personally do not need OOT build, but since it's in the code,
> I'd prefer either fix it or remove it, otherwise it's
> misleading. But I have not got reply from kselftest.
>
>  >>
>  >> >>>>> On Fri, 22 May 2020 07:13:02 +0300, Yauheni Kaliuta  wrote:
>  >>
>  >> > I had a look, here are some fixes.
>  >> > Yauheni Kaliuta (8):
>  >> >   selftests/bpf: remove test_align from Makefile
>  >> >   selftests/bpf: build bench.o for any $(OUTPUT)
>  >> >   selftests/bpf: install btf .c files
>  >> >   selftests/bpf: fix object files installation
>  >> >   selftests/bpf: add output dir to include list
>  >> >   selftests/bpf: fix urandom_read installation
>  >> >   selftests/bpf: fix test.h placing for out of tree build
>  >> >   selftests/bpf: factor out MKDIR rule
>  >>
>  >> >  tools/testing/selftests/bpf/Makefile | 77 ++++++++++++++++++++--------
>  >> >  1 file changed, 55 insertions(+), 22 deletions(-)
>  >>
>  >> > --
>  >> > 2.26.2
>  >>
>  >>
>  >> --
>  >> WBR,
>  >> Yauheni Kaliuta
>  >>
>
>
> --
> WBR,
> Yauheni Kaliuta
>
Yauheni Kaliuta May 27, 2020, 7:25 a.m. UTC | #7
Hi, Andrii!

>>>>> On Tue, 26 May 2020 22:04:35 -0700, Andrii Nakryiko  wrote:

 > On Tue, May 26, 2020 at 9:52 PM Yauheni Kaliuta
 > <yauheni.kaliuta@redhat.com> wrote:
 >> 
 >> Hi, Andrii!
 >> 
 >> >>>>> On Tue, 26 May 2020 15:32:10 -0700, Andrii Nakryiko  wrote:
 >> 
 >> > On Thu, May 21, 2020 at 11:41 PM Yauheni Kaliuta
 >> > <yauheni.kaliuta@redhat.com> wrote:
 >> >>
 >> >>
 >> >> Actually, a bit more needed :)
 >> 
 >> > From the other kselftest thread, it seems like selftests are not
 >> > supporting builds out-of-tree. With that, wouldn't it be simpler to
 >> > build in tree and then just copy selftests/bpf directory to wherever
 >> > you need to run tests from? It would be simple and reliable. Given I
 >> > and probably everyone else never build and run tests out-of-tree, it's
 >> > just too easy to break this and you'll be constantly chasing some
 >> > non-obvious breakages...
 >> 
 >> > Is there some problem with such approach?
 >> 
 >> This is `make install` ;).

 > So patch #2, #5, and #7 is solving just `make install` problem?..

No, they are fixing OOT build problems. I should have probably
split the series, oot and install fixes.

 > My point is that by building in tree and then just copying
 > everything under selftests/bpf directory to wherever you want
 > to "install" it would just work. And won't require
 > complicating already complicated Makefile. Any problem with
 > such approach?

I understand. I see only wasting of space as a problem, but
should check.

 >> 
 >> I personally do not need OOT build, but since it's in the code,
 >> I'd prefer either fix it or remove it, otherwise it's
 >> misleading. But I have not got reply from kselftest.
 >> 
 >> >>
 >> >> >>>>> On Fri, 22 May 2020 07:13:02 +0300, Yauheni Kaliuta  wrote:
 >> >>
 >> >> > I had a look, here are some fixes.
 >> >> > Yauheni Kaliuta (8):
 >> >> >   selftests/bpf: remove test_align from Makefile
 >> >> >   selftests/bpf: build bench.o for any $(OUTPUT)
 >> >> >   selftests/bpf: install btf .c files
 >> >> >   selftests/bpf: fix object files installation
 >> >> >   selftests/bpf: add output dir to include list
 >> >> >   selftests/bpf: fix urandom_read installation
 >> >> >   selftests/bpf: fix test.h placing for out of tree build
 >> >> >   selftests/bpf: factor out MKDIR rule
 >> >>
 >> >> >  tools/testing/selftests/bpf/Makefile | 77 ++++++++++++++++++++--------
 >> >> >  1 file changed, 55 insertions(+), 22 deletions(-)
 >> >>
 >> >> > --
 >> >> > 2.26.2
 >> >>
 >> >>
 >> >> --
 >> >> WBR,
 >> >> Yauheni Kaliuta
 >> >>
 >> 
 >> 
 >> --
 >> WBR,
 >> Yauheni Kaliuta
 >>
Yauheni Kaliuta May 27, 2020, 8:05 a.m. UTC | #8
On Wed, May 27, 2020 at 10:25 AM Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:

[...]

>  > My point is that by building in tree and then just copying
>  > everything under selftests/bpf directory to wherever you want
>  > to "install" it would just work. And won't require
>  > complicating already complicated Makefile. Any problem with
>  > such approach?
>
> I understand. I see only wasting of space as a problem, but
> should check.
>

Well, it messes up with the lib.mk functionality. There must be
explicit was for customization, like it's done with OVERRIDE_TARGETS.