Bpftool determines it's own version based on the kernel
version, which is picked from the linux/version.h header.
It's strange to use the version of the installed kernel
headers, and makes much more sense to use the version
of the actual source tree, where bpftool sources are.
This patch adds $(srctree)/usr/include to the list
of include files, which causes bpftool to use the version
from the source tree.
Example:
before:
$ bpftool version
bpftool v4.14.6
after:
$ bpftool version
bpftool v4.15.0
Signed-off-by: Roman Gushchin <[email protected]>
Cc: Jakub Kicinski <[email protected]>
Cc: Alexei Starovoitov <[email protected]>
Cc: Daniel Borkmann <[email protected]>
---
tools/bpf/bpftool/Makefile | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 9c089cfa5f3f..6864d416c49e 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -37,7 +37,9 @@ CC = gcc
CFLAGS += -O2
CFLAGS += -W -Wall -Wextra -Wno-unused-parameter -Wshadow
-CFLAGS += -D__EXPORTED_HEADERS__ -I$(srctree)/tools/include/uapi -I$(srctree)/tools/include -I$(srctree)/tools/lib/bpf -I$(srctree)/kernel/bpf/
+CFLAGS += -D__EXPORTED_HEADERS__ -I$(srctree)/tools/include/uapi
+CFLAGS += -I$(srctree)/tools/include -I$(srctree)/tools/lib/bpf
+CFLAGS += -I$(srctree)/kernel/bpf/ -I$(srctree)/usr/include
LIBS = -lelf -lbfd -lopcodes $(LIBBPF)
INSTALL ?= install
--
2.14.3
On 12/20/17 12:19 PM, Roman Gushchin wrote:
> Bpftool determines it's own version based on the kernel
> version, which is picked from the linux/version.h header.
>
> It's strange to use the version of the installed kernel
> headers, and makes much more sense to use the version
> of the actual source tree, where bpftool sources are.
>
> This patch adds $(srctree)/usr/include to the list
> of include files, which causes bpftool to use the version
> from the source tree.
>
> Example:
> before:
>
> $ bpftool version
> bpftool v4.14.6
>
> after:
> $ bpftool version
> bpftool v4.15.0
>
> Signed-off-by: Roman Gushchin <[email protected]>
> Cc: Jakub Kicinski <[email protected]>
> Cc: Alexei Starovoitov <[email protected]>
> Cc: Daniel Borkmann <[email protected]>
> ---
> tools/bpf/bpftool/Makefile | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index 9c089cfa5f3f..6864d416c49e 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -37,7 +37,9 @@ CC = gcc
>
> CFLAGS += -O2
> CFLAGS += -W -Wall -Wextra -Wno-unused-parameter -Wshadow
> -CFLAGS += -D__EXPORTED_HEADERS__ -I$(srctree)/tools/include/uapi -I$(srctree)/tools/include -I$(srctree)/tools/lib/bpf -I$(srctree)/kernel/bpf/
> +CFLAGS += -D__EXPORTED_HEADERS__ -I$(srctree)/tools/include/uapi
> +CFLAGS += -I$(srctree)/tools/include -I$(srctree)/tools/lib/bpf
> +CFLAGS += -I$(srctree)/kernel/bpf/ -I$(srctree)/usr/include
-I$(srctree)/usr/include may not work if build directory is not the same
as the source directory. You probably should use
-I$(objtree)/usr/include?
> LIBS = -lelf -lbfd -lopcodes $(LIBBPF)
>
> INSTALL ?= install
>
On Wed, 20 Dec 2017 20:19:43 +0000, Roman Gushchin wrote:
> Bpftool determines it's own version based on the kernel
> version, which is picked from the linux/version.h header.
>
> It's strange to use the version of the installed kernel
> headers, and makes much more sense to use the version
> of the actual source tree, where bpftool sources are.
>
> This patch adds $(srctree)/usr/include to the list
> of include files, which causes bpftool to use the version
> from the source tree.
>
> Example:
> before:
>
> $ bpftool version
> bpftool v4.14.6
>
> after:
> $ bpftool version
> bpftool v4.15.0
Thanks for the patch, this would indeed use some improvement.
How about we just run make to get the version like liblockdep does?
LIBLOCKDEP_VERSION=$(shell make --no-print-directory -sC ../../.. kernelversion)
probably s@../../..@$(srctree)@
$(srctree)/usr/include is not going to be there for out-of-source builds.
> Signed-off-by: Roman Gushchin <[email protected]>
> Cc: Jakub Kicinski <[email protected]>
> Cc: Alexei Starovoitov <[email protected]>
> Cc: Daniel Borkmann <[email protected]>
> ---
> tools/bpf/bpftool/Makefile | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index 9c089cfa5f3f..6864d416c49e 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -37,7 +37,9 @@ CC = gcc
>
> CFLAGS += -O2
> CFLAGS += -W -Wall -Wextra -Wno-unused-parameter -Wshadow
> -CFLAGS += -D__EXPORTED_HEADERS__ -I$(srctree)/tools/include/uapi -I$(srctree)/tools/include -I$(srctree)/tools/lib/bpf -I$(srctree)/kernel/bpf/
> +CFLAGS += -D__EXPORTED_HEADERS__ -I$(srctree)/tools/include/uapi
> +CFLAGS += -I$(srctree)/tools/include -I$(srctree)/tools/lib/bpf
> +CFLAGS += -I$(srctree)/kernel/bpf/ -I$(srctree)/usr/include
> LIBS = -lelf -lbfd -lopcodes $(LIBBPF)
>
> INSTALL ?= install
On Wed, Dec 20, 2017 at 12:29:21PM -0800, Jakub Kicinski wrote:
> On Wed, 20 Dec 2017 20:19:43 +0000, Roman Gushchin wrote:
> > Bpftool determines it's own version based on the kernel
> > version, which is picked from the linux/version.h header.
> >
> > It's strange to use the version of the installed kernel
> > headers, and makes much more sense to use the version
> > of the actual source tree, where bpftool sources are.
> >
> > This patch adds $(srctree)/usr/include to the list
> > of include files, which causes bpftool to use the version
> > from the source tree.
> >
> > Example:
> > before:
> >
> > $ bpftool version
> > bpftool v4.14.6
> >
> > after:
> > $ bpftool version
> > bpftool v4.15.0
>
> Thanks for the patch, this would indeed use some improvement.
>
> How about we just run make to get the version like liblockdep does?
>
> LIBLOCKDEP_VERSION=$(shell make --no-print-directory -sC ../../.. kernelversion)
>
> probably s@../../..@$(srctree)@
>
> $(srctree)/usr/include is not going to be there for out-of-source builds.
Hm, why it's better? It's not only about the kernel version,
IMO it's generally better to use includes from the source tree,
rather then system-wide installed kernel headers.
I've got about out-of-source builds, but do we support it in general?
How can I build bpftool outside of the kernel tree?
I've tried a bit, but failed.
Thank you!
On Wed, Dec 20, 2017 at 12:26:30PM -0800, Yonghong Song wrote:
>
>
> On 12/20/17 12:19 PM, Roman Gushchin wrote:
> > Bpftool determines it's own version based on the kernel
> > version, which is picked from the linux/version.h header.
> >
> > It's strange to use the version of the installed kernel
> > headers, and makes much more sense to use the version
> > of the actual source tree, where bpftool sources are.
> >
> > This patch adds $(srctree)/usr/include to the list
> > of include files, which causes bpftool to use the version
> > from the source tree.
> >
> > Example:
> > before:
> >
> > $ bpftool version
> > bpftool v4.14.6
> >
> > after:
> > $ bpftool version
> > bpftool v4.15.0
> >
> > Signed-off-by: Roman Gushchin <[email protected]>
> > Cc: Jakub Kicinski <[email protected]>
> > Cc: Alexei Starovoitov <[email protected]>
> > Cc: Daniel Borkmann <[email protected]>
> > ---
> > tools/bpf/bpftool/Makefile | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> > index 9c089cfa5f3f..6864d416c49e 100644
> > --- a/tools/bpf/bpftool/Makefile
> > +++ b/tools/bpf/bpftool/Makefile
> > @@ -37,7 +37,9 @@ CC = gcc
> > CFLAGS += -O2
> > CFLAGS += -W -Wall -Wextra -Wno-unused-parameter -Wshadow
> > -CFLAGS += -D__EXPORTED_HEADERS__ -I$(srctree)/tools/include/uapi -I$(srctree)/tools/include -I$(srctree)/tools/lib/bpf -I$(srctree)/kernel/bpf/
> > +CFLAGS += -D__EXPORTED_HEADERS__ -I$(srctree)/tools/include/uapi
> > +CFLAGS += -I$(srctree)/tools/include -I$(srctree)/tools/lib/bpf
> > +CFLAGS += -I$(srctree)/kernel/bpf/ -I$(srctree)/usr/include
>
> -I$(srctree)/usr/include may not work if build directory is not the same as
> the source directory. You probably should use
> -I$(objtree)/usr/include?
$(objtree) is not defined there, so it doesn't work.
Tbh, I struggle to say if it's supposed to work there or not.
Thanks!
On Wed, 20 Dec 2017 20:53:41 +0000, Roman Gushchin wrote:
> On Wed, Dec 20, 2017 at 12:29:21PM -0800, Jakub Kicinski wrote:
> > On Wed, 20 Dec 2017 20:19:43 +0000, Roman Gushchin wrote:
> > > Bpftool determines it's own version based on the kernel
> > > version, which is picked from the linux/version.h header.
> > >
> > > It's strange to use the version of the installed kernel
> > > headers, and makes much more sense to use the version
> > > of the actual source tree, where bpftool sources are.
> > >
> > > This patch adds $(srctree)/usr/include to the list
> > > of include files, which causes bpftool to use the version
> > > from the source tree.
> > >
> > > Example:
> > > before:
> > >
> > > $ bpftool version
> > > bpftool v4.14.6
> > >
> > > after:
> > > $ bpftool version
> > > bpftool v4.15.0
> >
> > Thanks for the patch, this would indeed use some improvement.
> >
> > How about we just run make to get the version like liblockdep does?
> >
> > LIBLOCKDEP_VERSION=$(shell make --no-print-directory -sC ../../.. kernelversion)
> >
> > probably s@../../..@$(srctree)@
> >
> > $(srctree)/usr/include is not going to be there for out-of-source builds.
>
> Hm, why it's better? It's not only about the kernel version,
> IMO it's generally better to use includes from the source tree,
> rather then system-wide installed kernel headers.
Right I agree the kernel headers are preferred. I'm not entirely sure
why we don't use them, if it was OK to assume usr/ is there we wouldn't
need the tools/include/uapi/ contraption. Maybe Arnaldo could explain?
> I've got about out-of-source builds, but do we support it in general?
> How can I build bpftool outside of the kernel tree?
> I've tried a bit, but failed.
This is what I do:
make -C tools/bpf/bpftool/ W=1 O=/tmp/builds/bpftool
> > > diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> > > index 9c089cfa5f3f..6864d416c49e 100644
> > > --- a/tools/bpf/bpftool/Makefile
> > > +++ b/tools/bpf/bpftool/Makefile
> > > @@ -37,7 +37,9 @@ CC = gcc
> > >
> > > CFLAGS += -O2
> > > CFLAGS += -W -Wall -Wextra -Wno-unused-parameter -Wshadow
> > > -CFLAGS += -D__EXPORTED_HEADERS__ -I$(srctree)/tools/include/uapi -I$(srctree)/> > > tools/include -I$(srctree)/tools/lib/bpf -I$(srctree)/kernel/bpf/
> > > +CFLAGS += -D__EXPORTED_HEADERS__ -I$(srctree)/tools/include/uapi
> > > +CFLAGS += -I$(srctree)/tools/include -I$(srctree)/tools/lib/bpf
> > > +CFLAGS += -I$(srctree)/kernel/bpf/ -I$(srctree)/usr/include
> > > LIBS = -lelf -lbfd -lopcodes $(LIBBPF)
> > >
> > > INSTALL ?= install
On Wed, Dec 20, 2017 at 01:52:18PM -0800, Jakub Kicinski wrote:
> On Wed, 20 Dec 2017 20:53:41 +0000, Roman Gushchin wrote:
> > On Wed, Dec 20, 2017 at 12:29:21PM -0800, Jakub Kicinski wrote:
> > > On Wed, 20 Dec 2017 20:19:43 +0000, Roman Gushchin wrote:
> > > > Bpftool determines it's own version based on the kernel
> > > > version, which is picked from the linux/version.h header.
> > > >
> > > > It's strange to use the version of the installed kernel
> > > > headers, and makes much more sense to use the version
> > > > of the actual source tree, where bpftool sources are.
> > > >
> > > > This patch adds $(srctree)/usr/include to the list
> > > > of include files, which causes bpftool to use the version
> > > > from the source tree.
> > > >
> > > > Example:
> > > > before:
> > > >
> > > > $ bpftool version
> > > > bpftool v4.14.6
> > > >
> > > > after:
> > > > $ bpftool version
> > > > bpftool v4.15.0
> > >
> > > Thanks for the patch, this would indeed use some improvement.
> > >
> > > How about we just run make to get the version like liblockdep does?
> > >
> > > LIBLOCKDEP_VERSION=$(shell make --no-print-directory -sC ../../.. kernelversion)
> > >
> > > probably s@../../..@$(srctree)@
> > >
> > > $(srctree)/usr/include is not going to be there for out-of-source builds.
> >
> > Hm, why it's better? It's not only about the kernel version,
> > IMO it's generally better to use includes from the source tree,
> > rather then system-wide installed kernel headers.
>
> Right I agree the kernel headers are preferred. I'm not entirely sure
> why we don't use them, if it was OK to assume usr/ is there we wouldn't
> need the tools/include/uapi/ contraption. Maybe Arnaldo could explain?
>
> > I've got about out-of-source builds, but do we support it in general?
> > How can I build bpftool outside of the kernel tree?
> > I've tried a bit, but failed.
>
> This is what I do:
>
> make -C tools/bpf/bpftool/ W=1 O=/tmp/builds/bpftool
This works perfectly with my patch:
$ make -C ~/linux/tools/bpf/ W=1 O=/home/guro/build/ --trace
<...>
echo ' CC '/home/guro/build/main.o;gcc -O2 -W -Wall -Wextra -Wno-unused-parameter -Wshadow -D__EXPORTED_HEADERS__ -I/home/guro/linux/tools/include/uapi -I/home/guro/linux/tools/include -I/home/guro/linux/tools/lib/bpf -I/home/guro/linux/kernel/bpf/ -I/home/guro/linux/usr/include -DNEW_DISSASSEMBLER_SIGNATURE -c -MMD -o /home/guro/build/main.o main.c
<...>
echo ' LINK '/home/guro/build/bpftool;gcc -O2 -W -Wall -Wextra -Wno-unused-parameter -Wshadow -D__EXPORTED_HEADERS__ -I/home/guro/linux/tools/include/uapi -I/home/guro/linux/tools/include -I/home/guro/linux/tools/lib/bpf -I/home/guro/linux/kernel/bpf/ -I/home/guro/linux/usr/include -DNEW_DISSASSEMBLER_SIGNATURE -o /home/guro/build/bpftool /home/guro/build/common.o /home/guro/build/cgroup.o /home/guro/build/main.o /home/guro/build/json_writer.o /home/guro/build/prog.o /home/guro/build/map.o /home/guro/build/jit_disasm.o /home/guro/build/disasm.o /home/guro/build/libbpf.a -lelf -lbfd -lopcodes /home/guro/build/libbpf.a
LINK /home/guro/build/bpftool
make[1]: Leaving directory '/home/guro/linux/tools/bpf/bpftool'
make: Leaving directory '/home/guro/linux/tools/bpf'
$ ./build/bpftool version
./build/bpftool v4.15.0
Thanks!
On Thu, 21 Dec 2017 12:07:42 +0000, Roman Gushchin wrote:
> On Wed, Dec 20, 2017 at 01:52:18PM -0800, Jakub Kicinski wrote:
> > On Wed, 20 Dec 2017 20:53:41 +0000, Roman Gushchin wrote:
> > > On Wed, Dec 20, 2017 at 12:29:21PM -0800, Jakub Kicinski wrote:
> > > Hm, why it's better? It's not only about the kernel version,
> > > IMO it's generally better to use includes from the source tree,
> > > rather then system-wide installed kernel headers.
> >
> > Right I agree the kernel headers are preferred. I'm not entirely sure
> > why we don't use them, if it was OK to assume usr/ is there we wouldn't
> > need the tools/include/uapi/ contraption. Maybe Arnaldo could explain?
> >
> > > I've got about out-of-source builds, but do we support it in general?
> > > How can I build bpftool outside of the kernel tree?
> > > I've tried a bit, but failed.
> >
> > This is what I do:
> >
> > make -C tools/bpf/bpftool/ W=1 O=/tmp/builds/bpftool
>
> This works perfectly with my patch:
>
> $ make -C ~/linux/tools/bpf/ W=1 O=/home/guro/build/ --trace
> <...>
> echo ' CC '/home/guro/build/main.o;gcc -O2 -W -Wall -Wextra -Wno-unused-parameter -Wshadow -D__EXPORTED_HEADERS__ -I/home/guro/linux/tools/include/uapi -I/home/guro/linux/tools/include -I/home/guro/linux/tools/lib/bpf -I/home/guro/linux/kernel/bpf/ -I/home/guro/linux/usr/include -DNEW_DISSASSEMBLER_SIGNATURE -c -MMD -o /home/guro/build/main.o main.c
> <...>
> echo ' LINK '/home/guro/build/bpftool;gcc -O2 -W -Wall -Wextra -Wno-unused-parameter -Wshadow -D__EXPORTED_HEADERS__ -I/home/guro/linux/tools/include/uapi -I/home/guro/linux/tools/include -I/home/guro/linux/tools/lib/bpf -I/home/guro/linux/kernel/bpf/ -I/home/guro/linux/usr/include -DNEW_DISSASSEMBLER_SIGNATURE -o /home/guro/build/bpftool /home/guro/build/common.o /home/guro/build/cgroup.o /home/guro/build/main.o /home/guro/build/json_writer.o /home/guro/build/prog.o /home/guro/build/map.o /home/guro/build/jit_disasm.o /home/guro/build/disasm.o /home/guro/build/libbpf.a -lelf -lbfd -lopcodes /home/guro/build/libbpf.a
> LINK /home/guro/build/bpftool
> make[1]: Leaving directory '/home/guro/linux/tools/bpf/bpftool'
> make: Leaving directory '/home/guro/linux/tools/bpf'
>
> $ ./build/bpftool version
> ./build/bpftool v4.15.0
Argh, sorry for the confusion you need to build the kernel out-of-source
as well. In my case I build the kernel and bpftool out of source, and
then the usr/ doesn't actually contain the auto-generated headers:
$ ls ~/devel/linux/usr/
gen_init_cpio.c initramfs_data.S Kconfig Makefile
Only build directory does:
$ ls /tmp/builds/usr/
built-in.o gen_init_cpio include initramfs_data.cpio initramfs_data.o modules.builtin modules.order
Let me reiterate, the user space headers we need should all be already
included in -I$(srctree)/tools/include/uapi, and make kernelversion is
nice because it also adds the -rc tags.