Re: [PATCH bpf-next v5 2/2] samples: bpf: refactor perf_event user program with libbpf bpf_link

Thank you for your time and effort for the review :)

Best,
Daniel

On Sun, Mar 22, 2020 at 4:43 AM Andrii Nakryiko
[off-list ref] wrote:

On Sat, Mar 21, 2020 at 3:04 AM Daniel T. Lee [off-list ref] wrote:

quoted

The bpf_program__attach of libbpf(using bpf_link) is much more intuitive
than the previous method using ioctl.

bpf_program__attach_perf_event manages the enable of perf_event and
attach of BPF programs to it, so there's no neeed to do this
directly with ioctl.

In addition, bpf_link provides consistency in the use of API because it
allows disable (detach, destroy) for multiple events to be treated as
one bpf_link__destroy. Also, bpf_link__destroy manages the close() of
perf_event fd.

This commit refactors samples that attach the bpf program to perf_event
by using libbbpf instead of ioctl. Also the bpf_load in the samples were
removed and migrated to use libbbpf API.

Signed-off-by: Daniel T. Lee <redacted>

---

LGTM.

Acked-by: Andrii Nakryiko <redacted>

quoted

Changes in v2:
 - check memory allocation is successful
 - clean up allocated memory on error

Changes in v3:
 - Improve pointer error check (IS_ERR())
 - change to calloc for easier destroy of bpf_link
 - remove perf_event fd list since bpf_link handles fd
 - use newer bpf_object__{open/load} API instead of bpf_prog_load
 - perf_event for _SC_NPROCESSORS_ONLN instead of _SC_NPROCESSORS_CONF
 - find program with name explicitly instead of bpf_program__next
 - unconditional bpf_link__destroy() on cleanup

Changes in v4:
 - bpf_link *, bpf_object * set NULL on init & err for easier destroy
 - close bpf object with bpf_object__close()

Changes in v5:
 - on error, return error code with exit

 samples/bpf/Makefile           |   4 +-
 samples/bpf/sampleip_user.c    |  98 +++++++++++++++--------
 samples/bpf/trace_event_user.c | 139 ++++++++++++++++++++++-----------
 3 files changed, 159 insertions(+), 82 deletions(-)

[...]