Message-ID: <20200117095721.0030f414@carbon>
Date:   Fri, 17 Jan 2020 09:57:21 +0100
From:   Jesper Dangaard Brouer <brouer@...hat.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Toke Høiland-Jørgensen <toke@...hat.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        Andrii Nakryiko <andriin@...com>,
        Doug Ledford <dledford@...hat.com>,
        Jason Gunthorpe <jgg@...pe.ca>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        John Fastabend <john.fastabend@...il.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Shuah Khan <shuah@...nel.org>, netdev@...r.kernel.org,
        bpf@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-rdma@...r.kernel.org, linux-kselftest@...r.kernel.org,
        clang-built-linux@...glegroups.com, brouer@...hat.com
Subject: Re: [PATCH bpf-next v3 00/11] tools: Use consistent libbpf include
 paths everywhere

On Thu, 16 Jan 2020 20:14:32 -0800
Alexei Starovoitov <alexei.starovoitov@...il.com> wrote:

> On Thu, Jan 16, 2020 at 02:22:11PM +0100, Toke Høiland-Jørgensen wrote:
> > 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.
> 
> > 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.


> 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.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

Powered by blists - more mailing lists