Thread (30 messages) 30 messages, 4 authors, 2020-01-17

Re: [PATCH bpf-next v3 00/11] tools: Use consistent libbpf include paths everywhere

From: Toke Høiland-Jørgensen <hidden>
Date: 2020-01-17 09:58:51
Also in: bpf, linux-kselftest, linux-rdma, lkml

Jesper Dangaard Brouer [off-list ref] writes:
On Thu, 16 Jan 2020 20:14:32 -0800
Alexei Starovoitov [off-list ref] wrote:
quoted
On Thu, Jan 16, 2020 at 02:22:11PM +0100, Toke Høiland-Jørgensen wrote:
quoted
The recent commit 6910d7d3867a ("selftests/bpf: Ensure bpf_helper_defs.h are
taken from selftests dir") broke compilation against libbpf if it is installed
on the system, and $INCLUDEDIR/bpf is not in the include path.

Since having the bpf/ subdir of $INCLUDEDIR in the include path has never been a
requirement for building against libbpf before, this needs to be fixed. One
option is to just revert the offending commit and figure out a different way to
achieve what it aims for.   
The offending commit has been in the tree for a week. So I applied Andrii's
revert of that change. It reintroduced the build dependency issue, but we lived
with it for long time, so we can take time to fix it cleanly.
I suggest to focus on that build dependency first.
quoted
However, this series takes a different approach:
Changing all in-tree users of libbpf to consistently use a bpf/ prefix in
#include directives for header files from libbpf.  
I'm not sure it's a good idea. It feels nice, but think of a message we're
sending to everyone. We will get spamed with question: does bpf community
require all libbpf users to use bpf/ prefix ? What should be our answer?
The answer should be: Yes. When libbpf install the header files the are
installed under bpf/ prefix.  It is very confusing that samples and
selftests can include libbpf.h without this prefix. Even worse
including "bpf.h" pickup the libbpf version bpf/bpf.h, which have
caused confusion.  The only reason for the direct "libbpf.h" include is
historical, as there used-to-be a local file for that.
Agreed. Also, we are already telling people what the right include path
is in at least two ways - and currently they are incompatible:

- The pkg-config file included with libbpf has a notion of include path;
  which does *not* include the bpf/ subdirectory.

- The skeleton generator puts an '#include <libbpf.h>' line into the
  generated files.

With this series we'll at least be consistent.
quoted
Require or recommend? If require.. what for? It works as-is. If recommend then
why suddenly we're changing all files in selftests and samples?
There is no good answer here. I think we should leave the things as-is.
I strongly believe we should correct this.  It doesn't make sense that
someone copying out a sample or selftests, into a git-submodule libbpf
(or distro installed libbpf-devel) have to understand that they have to
update the include path for all the libbpf header files.
Yeah, I think being clear and explicit about what is the recommended way
to include libbpf is strictly an improvement. And making it possible to
move example programs seamlessly in and out of the kernel tree will only
make things easier for people.

I'll rebase and respin this series on top of the revert (and fix
Andrii's comments).

-Toke
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help