From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>,
Andrii Nakryiko <andriin@fb.com>,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>
Subject: Re: [PATCH bpf-next] libbpf: Fix include of bpf_helpers.h when libbpf is installed on system
Date: Wed, 15 Jan 2020 09:39:32 +0100 [thread overview]
Message-ID: <87muapun97.fsf@toke.dk> (raw)
In-Reply-To: <20200114225857.kdmedok6mie55j6o@ast-mbp.dhcp.thefacebook.com>
Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> On Tue, Jan 14, 2020 at 10:26:57PM +0100, Toke Høiland-Jørgensen wrote:
>> Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:
>>
>> > On Tue, Jan 14, 2020 at 11:07 AM Andrii Nakryiko
>> > <andrii.nakryiko@gmail.com> wrote:
>> >>
>> >> On Tue, Jan 14, 2020 at 8:43 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>> >> >
>> >> > The change to use angled includes for bpf_helper_defs.h breaks compilation
>> >> > against libbpf when it is installed in the include path, since the file is
>> >> > installed in the bpf/ subdirectory of $INCLUDE_PATH. Fix this by adding the
>> >> > bpf/ prefix to the #include directive.
>> >> >
>> >> > Fixes: 6910d7d3867a ("selftests/bpf: Ensure bpf_helper_defs.h are taken from selftests dir")
>> >> > Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>> >> > ---
>> >> > Not actually sure this fix works for all the cases you originally tried to
>> >>
>> >> This does break selftests/bpf. Have you tried building selftests, does
>> >> it work for you? We need to fix selftests simultaneously with this
>> >> change.
>> >>
>> >> > fix with the referred commit; please check. Also, could we please stop breaking
>> >> > libbpf builds? :)
>> >>
>> >> Which libbpf build is failing right now? Both github and in-kernel
>> >> libbpf builds are fine. You must be referring to something else. What
>> >> exactly?
>> >
>> > I think it's better to just ensure that when compiling BPF programs,
>> > they have -I/usr/include/bpf specified, so that all BPF-side headers
>> > can be simply included as #include <bpf_helpers.h>, #include
>> > <bpf_tracing.h>, etc
>>
>> And break all programs that don't have that already? Just to make the
>> kernel build env slightly more convenient? Hardly friendly to the
>> library users, is it? :)
>
> Could you explain the breakage ?
> bpf_helpers.h and bpf_helper_defs.h are installed in the same location.
> If prefix==/usr during make install of libbpf they both will go into
> /usr/include/bpf
>
> Are you saying the bpf progs had:
> #include <bpf/bpf_helpers.h>
> to pick that header from /usr/include and commit 6910d7d3867a that did:
> +++ b/tools/lib/bpf/bpf_helpers.h
> @@ -2,7 +2,7 @@
> #ifndef __BPF_HELPERS__
> #define __BPF_HELPERS__
>
> -#include "bpf_helper_defs.h"
> +#include <bpf_helper_defs.h>
> broke it?
Yes. I get this error in xdp-tools[0] when updating lib/libbpf to the
latest upstream from github:
In file included from xdpfilt_blk_udp.c:10:
In file included from ./xdpfilt_prog.h:12:
../lib/libbpf/src/root/usr/include/bpf/bpf_helpers.h:5:10: error: 'bpf_helper_defs.h' file not found with <angled> include; use "quotes" instead
#include <bpf_helper_defs.h>
^~~~~~~~~~~~~~~~~~~
"bpf_helper_defs.h"
1 error generated.
> If so this bit needs to be reverted.
> And we need a selfttest for such include order.
Yes, please! :)
Trying to convince the selftests Makefile to do the right thing; will
send a v2 of this patch if I succeed...
-Toke
[0] https://github.com/xdp-project/xdp-tools
prev parent reply other threads:[~2020-01-15 8:39 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-14 16:42 [PATCH bpf-next] libbpf: Fix include of bpf_helpers.h when libbpf is installed on system Toke Høiland-Jørgensen
2020-01-14 19:07 ` Andrii Nakryiko
2020-01-14 19:41 ` Andrii Nakryiko
2020-01-14 21:26 ` Toke Høiland-Jørgensen
2020-01-14 22:58 ` Alexei Starovoitov
2020-01-15 8:39 ` Toke Høiland-Jørgensen [this message]
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=87muapun97.fsf@toke.dk \
--to=toke@redhat.com \
--cc=alexei.starovoitov@gmail.com \
--cc=andrii.nakryiko@gmail.com \
--cc=andriin@fb.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=netdev@vger.kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.