mbox series

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

Message ID 157918093154.1357254.7616059374996162336.stgit@toke.dk
Headers show
Series tools: Use consistent libbpf include paths everywhere | expand

Message

Toke Høiland-Jørgensen Jan. 16, 2020, 1:22 p.m. UTC 
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. 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.

This turns out to be a somewhat invasive change in the number of files touched;
however, the actual changes to files are fairly trivial (most of them are simply
made with 'sed'). Also, this approach has the advantage that it makes external
and internal users consistent with each other, and ensures no future changes
breaks things in the same way as the commit referenced above.

The series is split to make the change for one tool subdir at a time, while
trying not to break the build along the way. It is structured like this:

- Patch 1-3: Trivial fixes to Makefiles for issues I discovered while changing
  the include paths.

- Patch 4-8: Change the include directives to use the bpf/ prefix, and updates
  Makefiles to make sure tools/lib/ is part of the include path, but without
  removing tools/lib/bpf

- Patch 9-10: Remove tools/lib/bpf from include paths to make sure we don't
  inadvertently re-introduce includes without the bpf/ prefix.

- Patch 11: Change the bpf_helpers.h file in libbpf itself back to using a quoted
  include for bpf_helper_defs.h (the original source of breakage).

Changelog:

v3:
  - Don't add the kernel build dir to the runqslower Makefile, pass it in from
    selftests instead.
  - Use libbpf's 'make install_headers' in selftests instead of trying to
    generate bpf_helper_defs.h in-place (to also work on read-only filesystems).
  - Use a scratch builddir for both libbpf and bpftool when building in selftests.
  - Revert bpf_helpers.h to quoted include instead of angled include with a bpf/
    prefix.
  - Fix a few style nits from Andrii

v2:
  - Do a full cleanup of libbpf includes instead of just changing the
    bpf_helper_defs.h include.

---

Toke Høiland-Jørgensen (11):
      samples/bpf: Don't try to remove user's homedir on clean
      tools/bpf/runqslower: Fix override option for VMLINUX_BTF
      selftests: Pass VMLINUX_BTF to runqslower Makefile
      tools/runqslower: Use consistent include paths for libbpf
      selftests: Use consistent include paths for libbpf
      bpftool: Use consistent include paths for libbpf
      perf: Use consistent include paths for libbpf
      samples/bpf: Use consistent include paths for libbpf
      selftests: Remove tools/lib/bpf from include path
      tools/runqslower: Remove tools/lib/bpf from include path
      libbpf: Fix include of bpf_helpers.h when libbpf is installed on system


 samples/bpf/Makefile                               |    5 +-
 samples/bpf/cpustat_kern.c                         |    2 -
 samples/bpf/fds_example.c                          |    2 -
 samples/bpf/hbm.c                                  |    4 +-
 samples/bpf/hbm_kern.h                             |    4 +-
 samples/bpf/ibumad_kern.c                          |    2 -
 samples/bpf/ibumad_user.c                          |    2 -
 samples/bpf/lathist_kern.c                         |    2 -
 samples/bpf/lwt_len_hist_kern.c                    |    2 -
 samples/bpf/map_perf_test_kern.c                   |    4 +-
 samples/bpf/offwaketime_kern.c                     |    4 +-
 samples/bpf/offwaketime_user.c                     |    2 -
 samples/bpf/parse_ldabs.c                          |    2 -
 samples/bpf/parse_simple.c                         |    2 -
 samples/bpf/parse_varlen.c                         |    2 -
 samples/bpf/sampleip_kern.c                        |    4 +-
 samples/bpf/sampleip_user.c                        |    2 -
 samples/bpf/sock_flags_kern.c                      |    2 -
 samples/bpf/sockex1_kern.c                         |    2 -
 samples/bpf/sockex1_user.c                         |    2 -
 samples/bpf/sockex2_kern.c                         |    2 -
 samples/bpf/sockex2_user.c                         |    2 -
 samples/bpf/sockex3_kern.c                         |    2 -
 samples/bpf/spintest_kern.c                        |    4 +-
 samples/bpf/spintest_user.c                        |    2 -
 samples/bpf/syscall_tp_kern.c                      |    2 -
 samples/bpf/task_fd_query_kern.c                   |    2 -
 samples/bpf/task_fd_query_user.c                   |    2 -
 samples/bpf/tc_l2_redirect_kern.c                  |    2 -
 samples/bpf/tcbpf1_kern.c                          |    2 -
 samples/bpf/tcp_basertt_kern.c                     |    4 +-
 samples/bpf/tcp_bufs_kern.c                        |    4 +-
 samples/bpf/tcp_clamp_kern.c                       |    4 +-
 samples/bpf/tcp_cong_kern.c                        |    4 +-
 samples/bpf/tcp_dumpstats_kern.c                   |    4 +-
 samples/bpf/tcp_iw_kern.c                          |    4 +-
 samples/bpf/tcp_rwnd_kern.c                        |    4 +-
 samples/bpf/tcp_synrto_kern.c                      |    4 +-
 samples/bpf/tcp_tos_reflect_kern.c                 |    4 +-
 samples/bpf/test_cgrp2_tc_kern.c                   |    2 -
 samples/bpf/test_current_task_under_cgroup_kern.c  |    2 -
 samples/bpf/test_lwt_bpf.c                         |    2 -
 samples/bpf/test_map_in_map_kern.c                 |    4 +-
 samples/bpf/test_overhead_kprobe_kern.c            |    4 +-
 samples/bpf/test_overhead_raw_tp_kern.c            |    2 -
 samples/bpf/test_overhead_tp_kern.c                |    2 -
 samples/bpf/test_probe_write_user_kern.c           |    4 +-
 samples/bpf/trace_event_kern.c                     |    4 +-
 samples/bpf/trace_event_user.c                     |    2 -
 samples/bpf/trace_output_kern.c                    |    2 -
 samples/bpf/trace_output_user.c                    |    2 -
 samples/bpf/tracex1_kern.c                         |    4 +-
 samples/bpf/tracex2_kern.c                         |    4 +-
 samples/bpf/tracex3_kern.c                         |    4 +-
 samples/bpf/tracex4_kern.c                         |    4 +-
 samples/bpf/tracex5_kern.c                         |    4 +-
 samples/bpf/tracex6_kern.c                         |    2 -
 samples/bpf/tracex7_kern.c                         |    2 -
 samples/bpf/xdp1_kern.c                            |    2 -
 samples/bpf/xdp1_user.c                            |    4 +-
 samples/bpf/xdp2_kern.c                            |    2 -
 samples/bpf/xdp2skb_meta_kern.c                    |    2 -
 samples/bpf/xdp_adjust_tail_kern.c                 |    2 -
 samples/bpf/xdp_adjust_tail_user.c                 |    4 +-
 samples/bpf/xdp_fwd_kern.c                         |    2 -
 samples/bpf/xdp_fwd_user.c                         |    2 -
 samples/bpf/xdp_monitor_kern.c                     |    2 -
 samples/bpf/xdp_redirect_cpu_kern.c                |    2 -
 samples/bpf/xdp_redirect_cpu_user.c                |    2 -
 samples/bpf/xdp_redirect_kern.c                    |    2 -
 samples/bpf/xdp_redirect_map_kern.c                |    2 -
 samples/bpf/xdp_redirect_map_user.c                |    2 -
 samples/bpf/xdp_redirect_user.c                    |    2 -
 samples/bpf/xdp_router_ipv4_kern.c                 |    2 -
 samples/bpf/xdp_router_ipv4_user.c                 |    2 -
 samples/bpf/xdp_rxq_info_kern.c                    |    2 -
 samples/bpf/xdp_rxq_info_user.c                    |    4 +-
 samples/bpf/xdp_sample_pkts_kern.c                 |    2 -
 samples/bpf/xdp_sample_pkts_user.c                 |    2 -
 samples/bpf/xdp_tx_iptunnel_kern.c                 |    2 -
 samples/bpf/xdp_tx_iptunnel_user.c                 |    2 -
 samples/bpf/xdpsock_kern.c                         |    2 -
 samples/bpf/xdpsock_user.c                         |    6 +-
 tools/bpf/bpftool/Documentation/bpftool-gen.rst    |    2 -
 tools/bpf/bpftool/Makefile                         |    2 -
 tools/bpf/bpftool/btf.c                            |    8 ++-
 tools/bpf/bpftool/btf_dumper.c                     |    2 -
 tools/bpf/bpftool/cgroup.c                         |    2 -
 tools/bpf/bpftool/common.c                         |    4 +-
 tools/bpf/bpftool/feature.c                        |    4 +-
 tools/bpf/bpftool/gen.c                            |   10 ++--
 tools/bpf/bpftool/jit_disasm.c                     |    2 -
 tools/bpf/bpftool/main.c                           |    4 +-
 tools/bpf/bpftool/map.c                            |    4 +-
 tools/bpf/bpftool/map_perf_ring.c                  |    4 +-
 tools/bpf/bpftool/net.c                            |    8 ++-
 tools/bpf/bpftool/netlink_dumper.c                 |    4 +-
 tools/bpf/bpftool/perf.c                           |    2 -
 tools/bpf/bpftool/prog.c                           |    6 +-
 tools/bpf/bpftool/xlated_dumper.c                  |    2 -
 tools/bpf/runqslower/Makefile                      |   23 +++++----
 tools/bpf/runqslower/runqslower.bpf.c              |    2 -
 tools/bpf/runqslower/runqslower.c                  |    4 +-
 tools/lib/bpf/bpf_helpers.h                        |    2 -
 tools/perf/examples/bpf/5sec.c                     |    2 -
 tools/perf/examples/bpf/empty.c                    |    2 -
 tools/perf/examples/bpf/sys_enter_openat.c         |    2 -
 tools/perf/include/bpf/pid_filter.h                |    2 -
 tools/perf/include/bpf/stdio.h                     |    2 -
 tools/perf/include/bpf/unistd.h                    |    2 -
 tools/testing/selftests/bpf/.gitignore             |    1 
 tools/testing/selftests/bpf/Makefile               |   52 ++++++++++++--------
 tools/testing/selftests/bpf/bpf_tcp_helpers.h      |    4 +-
 tools/testing/selftests/bpf/bpf_trace_helpers.h    |    2 -
 tools/testing/selftests/bpf/bpf_util.h             |    2 -
 tools/testing/selftests/bpf/prog_tests/cpu_mask.c  |    2 -
 .../testing/selftests/bpf/prog_tests/perf_buffer.c |    2 -
 tools/testing/selftests/bpf/progs/bpf_dctcp.c      |    2 -
 tools/testing/selftests/bpf/progs/bpf_flow.c       |    4 +-
 tools/testing/selftests/bpf/progs/connect4_prog.c  |    4 +-
 tools/testing/selftests/bpf/progs/connect6_prog.c  |    4 +-
 tools/testing/selftests/bpf/progs/dev_cgroup.c     |    2 -
 tools/testing/selftests/bpf/progs/fentry_test.c    |    2 -
 tools/testing/selftests/bpf/progs/fexit_bpf2bpf.c  |    2 -
 .../selftests/bpf/progs/fexit_bpf2bpf_simple.c     |    2 -
 tools/testing/selftests/bpf/progs/fexit_test.c     |    2 -
 .../selftests/bpf/progs/get_cgroup_id_kern.c       |    2 -
 tools/testing/selftests/bpf/progs/kfree_skb.c      |    4 +-
 tools/testing/selftests/bpf/progs/loop1.c          |    4 +-
 tools/testing/selftests/bpf/progs/loop2.c          |    4 +-
 tools/testing/selftests/bpf/progs/loop3.c          |    4 +-
 tools/testing/selftests/bpf/progs/loop4.c          |    2 -
 tools/testing/selftests/bpf/progs/loop5.c          |    2 -
 tools/testing/selftests/bpf/progs/netcnt_prog.c    |    2 -
 tools/testing/selftests/bpf/progs/pyperf.h         |    2 -
 .../testing/selftests/bpf/progs/sample_map_ret0.c  |    2 -
 tools/testing/selftests/bpf/progs/sendmsg4_prog.c  |    4 +-
 tools/testing/selftests/bpf/progs/sendmsg6_prog.c  |    4 +-
 .../selftests/bpf/progs/socket_cookie_prog.c       |    4 +-
 .../selftests/bpf/progs/sockmap_parse_prog.c       |    4 +-
 .../selftests/bpf/progs/sockmap_tcp_msg_prog.c     |    4 +-
 .../selftests/bpf/progs/sockmap_verdict_prog.c     |    4 +-
 .../testing/selftests/bpf/progs/sockopt_inherit.c  |    2 -
 tools/testing/selftests/bpf/progs/sockopt_multi.c  |    2 -
 tools/testing/selftests/bpf/progs/sockopt_sk.c     |    2 -
 tools/testing/selftests/bpf/progs/strobemeta.h     |    2 -
 tools/testing/selftests/bpf/progs/tailcall1.c      |    2 -
 tools/testing/selftests/bpf/progs/tailcall2.c      |    2 -
 tools/testing/selftests/bpf/progs/tailcall3.c      |    2 -
 tools/testing/selftests/bpf/progs/tailcall4.c      |    2 -
 tools/testing/selftests/bpf/progs/tailcall5.c      |    2 -
 tools/testing/selftests/bpf/progs/tcp_rtt.c        |    2 -
 .../testing/selftests/bpf/progs/test_adjust_tail.c |    2 -
 .../selftests/bpf/progs/test_attach_probe.c        |    2 -
 tools/testing/selftests/bpf/progs/test_btf_haskv.c |    2 -
 tools/testing/selftests/bpf/progs/test_btf_newkv.c |    2 -
 tools/testing/selftests/bpf/progs/test_btf_nokv.c  |    2 -
 .../testing/selftests/bpf/progs/test_core_extern.c |    2 -
 .../selftests/bpf/progs/test_core_reloc_arrays.c   |    4 +-
 .../bpf/progs/test_core_reloc_bitfields_direct.c   |    4 +-
 .../bpf/progs/test_core_reloc_bitfields_probed.c   |    4 +-
 .../bpf/progs/test_core_reloc_existence.c          |    4 +-
 .../selftests/bpf/progs/test_core_reloc_flavors.c  |    4 +-
 .../selftests/bpf/progs/test_core_reloc_ints.c     |    4 +-
 .../selftests/bpf/progs/test_core_reloc_kernel.c   |    4 +-
 .../selftests/bpf/progs/test_core_reloc_misc.c     |    4 +-
 .../selftests/bpf/progs/test_core_reloc_mods.c     |    4 +-
 .../selftests/bpf/progs/test_core_reloc_nesting.c  |    4 +-
 .../bpf/progs/test_core_reloc_primitives.c         |    4 +-
 .../bpf/progs/test_core_reloc_ptr_as_arr.c         |    4 +-
 .../selftests/bpf/progs/test_core_reloc_size.c     |    4 +-
 .../selftests/bpf/progs/test_get_stack_rawtp.c     |    2 -
 .../testing/selftests/bpf/progs/test_global_data.c |    2 -
 .../selftests/bpf/progs/test_global_func1.c        |    2 -
 .../selftests/bpf/progs/test_global_func3.c        |    2 -
 .../selftests/bpf/progs/test_global_func5.c        |    2 -
 .../selftests/bpf/progs/test_global_func6.c        |    2 -
 .../selftests/bpf/progs/test_global_func7.c        |    2 -
 tools/testing/selftests/bpf/progs/test_l4lb.c      |    4 +-
 .../selftests/bpf/progs/test_l4lb_noinline.c       |    4 +-
 .../selftests/bpf/progs/test_lirc_mode2_kern.c     |    2 -
 .../selftests/bpf/progs/test_lwt_ip_encap.c        |    4 +-
 .../selftests/bpf/progs/test_lwt_seg6local.c       |    4 +-
 .../testing/selftests/bpf/progs/test_map_in_map.c  |    2 -
 tools/testing/selftests/bpf/progs/test_map_lock.c  |    2 -
 tools/testing/selftests/bpf/progs/test_mmap.c      |    2 -
 tools/testing/selftests/bpf/progs/test_obj_id.c    |    2 -
 tools/testing/selftests/bpf/progs/test_overhead.c  |    4 +-
 .../testing/selftests/bpf/progs/test_perf_buffer.c |    2 -
 tools/testing/selftests/bpf/progs/test_pinning.c   |    2 -
 .../selftests/bpf/progs/test_pinning_invalid.c     |    2 -
 .../testing/selftests/bpf/progs/test_pkt_access.c  |    4 +-
 .../selftests/bpf/progs/test_pkt_md_access.c       |    2 -
 .../testing/selftests/bpf/progs/test_probe_user.c  |    4 +-
 .../selftests/bpf/progs/test_queue_stack_map.h     |    2 -
 .../testing/selftests/bpf/progs/test_rdonly_maps.c |    2 -
 tools/testing/selftests/bpf/progs/test_seg6_loop.c |    4 +-
 .../bpf/progs/test_select_reuseport_kern.c         |    4 +-
 .../selftests/bpf/progs/test_send_signal_kern.c    |    2 -
 .../selftests/bpf/progs/test_sk_lookup_kern.c      |    4 +-
 .../selftests/bpf/progs/test_skb_cgroup_id_kern.c  |    2 -
 tools/testing/selftests/bpf/progs/test_skb_ctx.c   |    2 -
 tools/testing/selftests/bpf/progs/test_skeleton.c  |    2 -
 .../selftests/bpf/progs/test_sock_fields_kern.c    |    4 +-
 tools/testing/selftests/bpf/progs/test_spin_lock.c |    2 -
 .../selftests/bpf/progs/test_stacktrace_build_id.c |    2 -
 .../selftests/bpf/progs/test_stacktrace_map.c      |    2 -
 .../selftests/bpf/progs/test_sysctl_loop1.c        |    2 -
 .../selftests/bpf/progs/test_sysctl_loop2.c        |    2 -
 .../testing/selftests/bpf/progs/test_sysctl_prog.c |    2 -
 tools/testing/selftests/bpf/progs/test_tc_edt.c    |    4 +-
 tools/testing/selftests/bpf/progs/test_tc_tunnel.c |    4 +-
 .../bpf/progs/test_tcp_check_syncookie_kern.c      |    4 +-
 .../testing/selftests/bpf/progs/test_tcp_estats.c  |    2 -
 .../testing/selftests/bpf/progs/test_tcpbpf_kern.c |    4 +-
 .../selftests/bpf/progs/test_tcpnotify_kern.c      |    4 +-
 .../testing/selftests/bpf/progs/test_tracepoint.c  |    2 -
 .../testing/selftests/bpf/progs/test_tunnel_kern.c |    4 +-
 .../selftests/bpf/progs/test_verif_scale1.c        |    2 -
 .../selftests/bpf/progs/test_verif_scale2.c        |    2 -
 .../selftests/bpf/progs/test_verif_scale3.c        |    2 -
 tools/testing/selftests/bpf/progs/test_xdp.c       |    4 +-
 tools/testing/selftests/bpf/progs/test_xdp_loop.c  |    4 +-
 tools/testing/selftests/bpf/progs/test_xdp_meta.c  |    2 -
 .../selftests/bpf/progs/test_xdp_noinline.c        |    4 +-
 .../selftests/bpf/progs/test_xdp_redirect.c        |    2 -
 tools/testing/selftests/bpf/progs/test_xdp_vlan.c  |    4 +-
 tools/testing/selftests/bpf/progs/xdp_dummy.c      |    2 -
 .../testing/selftests/bpf/progs/xdp_redirect_map.c |    2 -
 tools/testing/selftests/bpf/progs/xdp_tx.c         |    2 -
 tools/testing/selftests/bpf/progs/xdping_kern.c    |    4 +-
 tools/testing/selftests/bpf/test_cpp.cpp           |    6 +-
 tools/testing/selftests/bpf/test_hashmap.c         |    2 -
 tools/testing/selftests/bpf/test_progs.h           |    2 -
 tools/testing/selftests/bpf/test_sock.c            |    2 -
 tools/testing/selftests/bpf/test_sockmap_kern.h    |    4 +-
 tools/testing/selftests/bpf/test_sysctl.c          |    2 -
 tools/testing/selftests/bpf/trace_helpers.h        |    2 -
 238 files changed, 381 insertions(+), 368 deletions(-)

Comments

Alexei Starovoitov Jan. 17, 2020, 4:14 a.m. UTC | #1 
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?
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.
And fix build dep differently.

Patches 1-3 are still worth doing.
Jesper Dangaard Brouer Jan. 17, 2020, 8:57 a.m. UTC | #2 
On Thu, 16 Jan 2020 20:14:32 -0800
Alexei Starovoitov <alexei.starovoitov@gmail.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.
Toke Høiland-Jørgensen Jan. 17, 2020, 9:58 a.m. UTC | #3 
Jesper Dangaard Brouer <brouer@redhat.com> writes:

> On Thu, 16 Jan 2020 20:14:32 -0800
> Alexei Starovoitov <alexei.starovoitov@gmail.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.

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.

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