linux-next: build failure after merge of the net tree

Hi all,

After merging the net tree, today's linux-next build (powerpc64le perf)
failed like this:

Warning: tools/include/uapi/linux/bpf.h differs from kernel
bpf.c: In function 'bpf_prog_attach':
bpf.c:180:6: error: 'union bpf_attr' has no member named 'attach_flags'; did you mean 'map_flags'?
  attr.attach_flags  = flags;
      ^

Caused by commit

  7f677633379b ("bpf: introduce BPF_F_ALLOW_OVERRIDE flag")

Unfortunately, the perf header files are kept separate from the kernel
header files proper and are not automatically copied over :-(

I have applied the following build fix patch for today.

From: Stephen Rothwell <redacted>
Date: Tue, 14 Feb 2017 08:22:20 +1100
Subject: [PATCH] bpf: kernel header files need to be copied into the tools directory

Signed-off-by: Stephen Rothwell <redacted>
---
 tools/include/uapi/linux/bpf.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 0eb0e87dbe9f..d2b0ac799d03 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h

@@ -116,6 +116,12 @@ enum bpf_attach_type {
 
 #define MAX_BPF_ATTACH_TYPE __MAX_BPF_ATTACH_TYPE
 
+/* If BPF_F_ALLOW_OVERRIDE flag is used in BPF_PROG_ATTACH command
+ * to the given target_fd cgroup the descendent cgroup will be able to
+ * override effective bpf program that was inherited from this cgroup
+ */
+#define BPF_F_ALLOW_OVERRIDE	(1U << 0)
+
 #define BPF_PSEUDO_MAP_FD	1
 
 /* flags for BPF_MAP_UPDATE_ELEM command */

@@ -171,6 +177,7 @@ union bpf_attr {
 		__u32		target_fd;	/* container object to attach to */
 		__u32		attach_bpf_fd;	/* eBPF program to attach */
 		__u32		attach_type;
+		__u32		attach_flags;
 	};
 } __attribute__((aligned(8)));
 

-- 
2.10.2

-- 
Cheers,
Stephen Rothwell

On 2/13/17 2:12 PM, Stephen Rothwell wrote:

Hi all,

After merging the net tree, today's linux-next build (powerpc64le perf)
failed like this:

Warning: tools/include/uapi/linux/bpf.h differs from kernel
bpf.c: In function 'bpf_prog_attach':
bpf.c:180:6: error: 'union bpf_attr' has no member named 'attach_flags'; did you mean 'map_flags'?
   attr.attach_flags  = flags;
       ^

Caused by commit

   7f677633379b ("bpf: introduce BPF_F_ALLOW_OVERRIDE flag")

Unfortunately, the perf header files are kept separate from the kernel
header files proper and are not automatically copied over :-(

I have applied the following build fix patch for today.

Yes. Thanks for the fix. It's more than a merge conflict.
I should have added it in the first place. Now we have both
perf and samples/bpf depend on tools/lib/bpf and I simply
forgot about this dependency, since building perf
is not my typical workflow.

Joe,
can you think of a way to make tools/lib/bpf to
use tools/include only?
Right now we just pull tools/lib/bpf/bpf.o in samples/bpf/Makefile
and that's a hack that caused this issue.
samples/bpf/ needs to depend on libbpf.a properly.

For the patch:
Acked-by: Alexei Starovoitov <ast@kernel.org>

Dave,
can you apply it to 'net' tree,
since the patch properly made it into patchwork ?
or I can resubmit it.
Thanks!

* Stephen Rothwell [off-list ref] wrote:

BTW., guys, for heaven's sake, please use the standard (multi-line) comment style:

  /*
   * Comment .....
   * ...... goes here.
   */

specified in Documentation/CodingStyle...

It's not that hard to create visually balanced patterns.

Thanks,

        Ingo

* Stephen Rothwell [off-list ref] wrote:

Hi all,

After merging the net tree, today's linux-next build (powerpc64le perf)
failed like this:

Warning: tools/include/uapi/linux/bpf.h differs from kernel
bpf.c: In function 'bpf_prog_attach':
bpf.c:180:6: error: 'union bpf_attr' has no member named 'attach_flags'; did you mean 'map_flags'?
  attr.attach_flags  = flags;
      ^

Caused by commit

  7f677633379b ("bpf: introduce BPF_F_ALLOW_OVERRIDE flag")

Unfortunately, the perf header files are kept separate from the kernel
header files proper and are not automatically copied over :-(

No, that's wrong, the problem is not that headers were not shared, the problem is 
that a tooling interdependency was not properly tested *and* that the dependency 
was not properly implemented in the build system either.

Note that we had similar build breakages when include headers _were_ shared as 
well, so sharing the headers would only have worked around this particular bug and 
would have introduced fragility in other places...

The best, most robust solution in this particular case would be to fix the 
(tooling) build system to express the dependency, that would have shown the build 
failure right when the modification was done.

Thanks,

	Ingo

On 13 February 2017 at 15:31, Alexei Starovoitov [off-list ref] wrote:

On 2/13/17 2:12 PM, Stephen Rothwell wrote:

quoted

Hi all,

After merging the net tree, today's linux-next build (powerpc64le perf)
failed like this:

Warning: tools/include/uapi/linux/bpf.h differs from kernel
bpf.c: In function 'bpf_prog_attach':
bpf.c:180:6: error: 'union bpf_attr' has no member named 'attach_flags';
did you mean 'map_flags'?
   attr.attach_flags  = flags;
       ^

Caused by commit

   7f677633379b ("bpf: introduce BPF_F_ALLOW_OVERRIDE flag")

Unfortunately, the perf header files are kept separate from the kernel
header files proper and are not automatically copied over :-(

I have applied the following build fix patch for today.

Yes. Thanks for the fix. It's more than a merge conflict.
I should have added it in the first place. Now we have both
perf and samples/bpf depend on tools/lib/bpf and I simply
forgot about this dependency, since building perf
is not my typical workflow.

Joe,
can you think of a way to make tools/lib/bpf to
use tools/include only?
Right now we just pull tools/lib/bpf/bpf.o in samples/bpf/Makefile
and that's a hack that caused this issue.
samples/bpf/ needs to depend on libbpf.a properly.

Honestly the build system stuff is all black magic to me.

Originally I had a line in the samples/bpf makefile that changed
directory and ran make there, but it was limited in that it only
addressed the 'all' target so you could still hit issues when trying
to build specific samples directly via their own targets so I removed
it on request:

https://patchwork.kernel.org/patch/9472573/

On Tue, Feb 14, 2017 at 07:42:21AM +0100, Ingo Molnar wrote:

* Stephen Rothwell [off-list ref] wrote:

quoted

Hi all,

After merging the net tree, today's linux-next build (powerpc64le perf)
failed like this:

Warning: tools/include/uapi/linux/bpf.h differs from kernel
bpf.c: In function 'bpf_prog_attach':
bpf.c:180:6: error: 'union bpf_attr' has no member named 'attach_flags'; did you mean 'map_flags'?
  attr.attach_flags  = flags;
      ^

Caused by commit

  7f677633379b ("bpf: introduce BPF_F_ALLOW_OVERRIDE flag")

Unfortunately, the perf header files are kept separate from the kernel
header files proper and are not automatically copied over :-(

No, that's wrong, the problem is not that headers were not shared, the problem is 
that a tooling interdependency was not properly tested *and* that the dependency 
was not properly implemented in the build system either.

Note that we had similar build breakages when include headers _were_ shared as 
well, so sharing the headers would only have worked around this particular bug and 
would have introduced fragility in other places...

The best, most robust solution in this particular case would be to fix the 
(tooling) build system to express the dependency, that would have shown the build 
failure right when the modification was done.

so we have the warning now:
  Warning: tools/include/uapi/linux/bpf.h differs from kernel

do you want to change it into the build failure?

jirka

Em Tue, Feb 14, 2017 at 10:19:37AM +0100, Jiri Olsa escreveu:

On Tue, Feb 14, 2017 at 07:42:21AM +0100, Ingo Molnar wrote:

quoted

* Stephen Rothwell [off-list ref] wrote:

quoted

Unfortunately, the perf header files are kept separate from the kernel
header files proper and are not automatically copied over :-(

quoted

No, that's wrong, the problem is not that headers were not shared, the problem is 
that a tooling interdependency was not properly tested *and* that the dependency 
was not properly implemented in the build system either.

quoted

Note that we had similar build breakages when include headers _were_ shared as 
well, so sharing the headers would only have worked around this particular bug and 
would have introduced fragility in other places...

quoted

The best, most robust solution in this particular case would be to fix the 
(tooling) build system to express the dependency, that would have shown the build 
failure right when the modification was done.

so we have the warning now:
  Warning: tools/include/uapi/linux/bpf.h differs from kernel

do you want to change it into the build failure?

No. Differences in the copy are not always problematic, the problem here
lies elsewhere.

Please run:

  make -C tools all

To build all tools when you touch something in tools/include and/or
tools/lib/

- Arnaldo



Bored? Here is what I first wrote ;-)

Simply using the kernel original would require kernel hackers to build
all tools using that file, something we long decided not to do.

What I think Ingo meant with dependency at the build system level is to
somehow state that if file A gets changed, then tool B must be rebuilt.

Now that samples/bpf and tools/perf/ depend on tools/lib/bpf/ I _always_
build both, ditto for tools/objtool, that shares a different library
with tools/perf/, tools/lib/subcmd/:

ENTRYPOINT make -C /git/linux/tools/perf O=/tmp/build/perf && \
           rm -rf /tmp/build/perf/{.[^.]*,*} && \
           make NO_LIBELF=1 -C /git/linux/tools/perf O=/tmp/build/perf && \
           make -C /git/linux/tools/objtool O=/tmp/build/objtool && \
           make -C /git/linux O=/tmp/build/linux allmodconfig && \
           make -C /git/linux O=/tmp/build/linux headers_install && \
           make -C /git/linux O=/tmp/build/linux samples/bpf/

This is the default action for my
docker.io/acmel/linux-perf-tools-build-fedora:rawhide container.

It is published, so a:

   docker pull docker.io/acmel/linux-perf-tools-build-fedora:rawhide

And then run it before pushing things upstream would catch these kinds
of errors.

But that would possibly disrupt too much people's workflow, that is why
using the Kbuild originated tools/build/ we have to somehow express that
when a change is made in a file then a tool that uses that file needs to
be rebuilt.

Makefile rules probably would be enough, but then it would have to be
done at the tools/build/ level and all tools using shared components
would have to use it to trigger the rebuild.

- Arnaldo

On Tue, Feb 14, 2017 at 09:50:20AM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

What I think Ingo meant with dependency at the build system level is to
somehow state that if file A gets changed, then tool B must be rebuilt.

Now that samples/bpf and tools/perf/ depend on tools/lib/bpf/ I _always_
build both, ditto for tools/objtool, that shares a different library
with tools/perf/, tools/lib/subcmd/:

ENTRYPOINT make -C /git/linux/tools/perf O=/tmp/build/perf && \
           rm -rf /tmp/build/perf/{.[^.]*,*} && \
           make NO_LIBELF=1 -C /git/linux/tools/perf O=/tmp/build/perf && \
           make -C /git/linux/tools/objtool O=/tmp/build/objtool && \
           make -C /git/linux O=/tmp/build/linux allmodconfig && \
           make -C /git/linux O=/tmp/build/linux headers_install && \
           make -C /git/linux O=/tmp/build/linux samples/bpf/

This is the default action for my
docker.io/acmel/linux-perf-tools-build-fedora:rawhide container.

It is published, so a:

   docker pull docker.io/acmel/linux-perf-tools-build-fedora:rawhide

And then run it before pushing things upstream would catch these kinds
of errors.

But that would possibly disrupt too much people's workflow, that is why
using the Kbuild originated tools/build/ we have to somehow express that
when a change is made in a file then a tool that uses that file needs to
be rebuilt.

we already have the check in the check-headers.sh script,
an AFAICS there's no 'rebuild' option here.. just warn or fail
because the headers update needs to be done manualy

Makefile rules probably would be enough, but then it would have to be
done at the tools/build/ level and all tools using shared components
would have to use it to trigger the rebuild.

we can move/invoke the check-headers.sh script in some upper dir

jirka

Em Tue, Feb 14, 2017 at 02:23:26PM +0100, Jiri Olsa escreveu:

On Tue, Feb 14, 2017 at 09:50:20AM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

quoted

What I think Ingo meant with dependency at the build system level is to
somehow state that if file A gets changed, then tool B must be rebuilt.

Now that samples/bpf and tools/perf/ depend on tools/lib/bpf/ I _always_
build both, ditto for tools/objtool, that shares a different library
with tools/perf/, tools/lib/subcmd/:

ENTRYPOINT make -C /git/linux/tools/perf O=/tmp/build/perf && \
           rm -rf /tmp/build/perf/{.[^.]*,*} && \
           make NO_LIBELF=1 -C /git/linux/tools/perf O=/tmp/build/perf && \
           make -C /git/linux/tools/objtool O=/tmp/build/objtool && \
           make -C /git/linux O=/tmp/build/linux allmodconfig && \
           make -C /git/linux O=/tmp/build/linux headers_install && \
           make -C /git/linux O=/tmp/build/linux samples/bpf/

This is the default action for my
docker.io/acmel/linux-perf-tools-build-fedora:rawhide container.

It is published, so a:

   docker pull docker.io/acmel/linux-perf-tools-build-fedora:rawhide

And then run it before pushing things upstream would catch these kinds
of errors.

But that would possibly disrupt too much people's workflow, that is why
using the Kbuild originated tools/build/ we have to somehow express that
when a change is made in a file then a tool that uses that file needs to
be rebuilt.

we already have the check in the check-headers.sh script,
an AFAICS there's no 'rebuild' option here.. just warn or fail
because the headers update needs to be done manualy

... when needed. And that will only be detected if you try to build
tools using what is in tools/include/linux/bpf.h

Tools using tools/lib/bpf/ _must_ use what is in tools/include/.

So lemme see if my reasoning is right:

tools/lib/bpf/bpf.c has:

  #include <linux/bpf.h>

Now, samples/bpf/ will build tools/lib/bpf/bpf.o:

# Libbpf dependencies
LIBBPF := ../../tools/lib/bpf/bpf.o

HOSTCFLAGS += -I$(objtree)/usr/include
HOSTCFLAGS += -I$(srctree)/tools/lib/
HOSTCFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
HOSTCFLAGS += -I$(srctree)/tools/lib/ -I$(srctree)/tools/include
HOSTCFLAGS += -I$(srctree)/tools/perf

HOSTCFLAGS_bpf_load.o += -I$(objtree)/usr/include -Wno-unused-variable

So it will never include tools/include/uapi/linux/bpf.h, which it
should.

Because the workflow people working on sample/bpf/ is to first install
the new headers using a variation of:

  make headers_install

So they will get the new bpf.h, not use tools/include/uapi/linux/bpf.h,
b00m.

They should use tools/include/uapi/linux/bpf.h, which is the one we know
builds well with tools/lib/bpf/bpf.c, since we tested it last time we
made the copy.
 

quoted

Makefile rules probably would be enough, but then it would have to be
done at the tools/build/ level and all tools using shared components
would have to use it to trigger the rebuild.

we can move/invoke the check-headers.sh script in some upper dir

Most of the time I just ignore that warning, only when I find spare time
I go look if the changes in the kernel copy, i.e. upstream, should
trigger changes in the tools using its copy in tools/include/.

- Arnaldo

Hi all,

On Tue, 14 Feb 2017 09:12:50 +1100 Stephen Rothwell [off-list ref] wrote:

After merging the net tree, today's linux-next build (powerpc64le perf)
failed like this:

Warning: tools/include/uapi/linux/bpf.h differs from kernel
bpf.c: In function 'bpf_prog_attach':
bpf.c:180:6: error: 'union bpf_attr' has no member named 'attach_flags'; did you mean 'map_flags'?
  attr.attach_flags  = flags;
      ^

Caused by commit

  7f677633379b ("bpf: introduce BPF_F_ALLOW_OVERRIDE flag")

So do we have a fix for this?  I am sure that Dave would like to send
his "net" tree to Linus sometime soonish ...

-- 
Cheers,
Stephen Rothwell

On 2/15/17 7:02 PM, Stephen Rothwell wrote:

Hi all,

On Tue, 14 Feb 2017 09:12:50 +1100 Stephen Rothwell [off-list ref] wrote:

quoted

After merging the net tree, today's linux-next build (powerpc64le perf)
failed like this:

Warning: tools/include/uapi/linux/bpf.h differs from kernel
bpf.c: In function 'bpf_prog_attach':
bpf.c:180:6: error: 'union bpf_attr' has no member named 'attach_flags'; did you mean 'map_flags'?
   attr.attach_flags  = flags;
       ^

Caused by commit

   7f677633379b ("bpf: introduce BPF_F_ALLOW_OVERRIDE flag")

So do we have a fix for this?  I am sure that Dave would like to send
his "net" tree to Linus sometime soonish ...

Do you mind resending it to netdev with my Ack ?
please mention [PATCH net] in subj, so it's get caught by Dave's scripts.

From: Alexei Starovoitov <redacted>
Date: Wed, 15 Feb 2017 19:06:02 -0800

On 2/15/17 7:02 PM, Stephen Rothwell wrote:

quoted

Hi all,

On Tue, 14 Feb 2017 09:12:50 +1100 Stephen Rothwell
[off-list ref] wrote:

quoted

After merging the net tree, today's linux-next build (powerpc64le
perf)
failed like this:

Warning: tools/include/uapi/linux/bpf.h differs from kernel
bpf.c: In function 'bpf_prog_attach':
bpf.c:180:6: error: 'union bpf_attr' has no member named
'attach_flags'; did you mean 'map_flags'?
   attr.attach_flags  = flags;
       ^

Caused by commit

   7f677633379b ("bpf: introduce BPF_F_ALLOW_OVERRIDE flag")

So do we have a fix for this?  I am sure that Dave would like to send
his "net" tree to Linus sometime soonish ...

Do you mind resending it to netdev with my Ack ?
please mention [PATCH net] in subj, so it's get caught by Dave's
scripts.

I applied the fix that synced the two header files already.

On 2/15/17 7:27 PM, David Miller wrote:

From: Alexei Starovoitov <redacted>
Date: Wed, 15 Feb 2017 19:06:02 -0800

quoted

On 2/15/17 7:02 PM, Stephen Rothwell wrote:

quoted

Hi all,

On Tue, 14 Feb 2017 09:12:50 +1100 Stephen Rothwell
[off-list ref] wrote:

quoted

After merging the net tree, today's linux-next build (powerpc64le
perf)
failed like this:

Warning: tools/include/uapi/linux/bpf.h differs from kernel
bpf.c: In function 'bpf_prog_attach':
bpf.c:180:6: error: 'union bpf_attr' has no member named
'attach_flags'; did you mean 'map_flags'?
    attr.attach_flags  = flags;
        ^

Caused by commit

    7f677633379b ("bpf: introduce BPF_F_ALLOW_OVERRIDE flag")

So do we have a fix for this?  I am sure that Dave would like to send
his "net" tree to Linus sometime soonish ...

Do you mind resending it to netdev with my Ack ?
please mention [PATCH net] in subj, so it's get caught by Dave's
scripts.

I applied the fix that synced the two header files already.

Ahh. Thanks!

Hi Dave,

On Wed, 15 Feb 2017 22:27:46 -0500 (EST) David Miller [off-list ref] wrote:

I applied the fix that synced the two header files already.

Sorry about that, I missed it in checking your tree over breakfast :-)

-- 
Cheers,
Stephen Rothwell