Re: [PATCH v1 1/1] libbpf: perfbuf: allow raw access to buffers

[Jon Doron <arilou@gmail.com>]

On 07/07/2022, Song Liu wrote:

Hi Song Liu, thank you for the fast reply, I'll add comments on top
of your original email.

I will not be available through out next week, so sorry if I'll have a 
late reply for follow ups.

Thanks in advance,
-- Jon.

>On Thu, Jul 7, 2022 at 12:14 AM Jon Doron <arilou@gmail.com> wrote:
>
>Please prefix the patch with the target tree (bpf or bpf-next). For example,
>this patch should go via bpf-next.

Done

>>
>> From: Jon Doron <jond@wiz.io>
>>
>> Add API for perfbuf to support writing a custom event reader.
>
>This is too brief for such change. It is ok to duplicate text from the cover
>letter.
>

Done

>Please also update libbpf.map.
>

Done

>Also, we should add a selftest to use these new APIs.
>

I'm not sure I'm following here what were you had in mind, in practice I 
could just change **bpf_perf_event_read_simple** implementation to use
these new APIs to initialize it's variables (data_head, data_tail, 
base, mmap_size) and it would act exactly the same.

>>
>> Signed-off-by: Jon Doron <jond@wiz.io>
>> ---
>>  tools/lib/bpf/libbpf.c | 40 ++++++++++++++++++++++++++++++++++++++++
>>  tools/lib/bpf/libbpf.h |  6 ++++++
>>  2 files changed, 46 insertions(+)
>>
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index e89cc9c885b3..37299aa05185 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -12433,6 +12433,46 @@ static int perf_buffer__process_records(struct perf_buffer *pb,
>>         return 0;
>>  }
>>
>> +int perf_buffer__raw_ring_buf(const struct perf_buffer *pb, size_t buf_idx,
>> +                             void **base, size_t *buf_size, __u64 *head,
>> +                             __u64 *tail)
>
>Please add comments to each API function.

Done

>> +{
>> +       struct perf_cpu_buf *cpu_buf;
>> +       struct perf_event_mmap_page *header;
>> +
>> +       if (buf_idx >= pb->cpu_cnt)
>> +               return libbpf_err(-EINVAL);
>> +
>> +       cpu_buf = pb->cpu_bufs[buf_idx];
>> +       if (!cpu_buf)
>> +               return libbpf_err(-ENOENT);
>> +
>> +       header = cpu_buf->base;
>> +       *head = ring_buffer_read_head(header);
>> +       *tail = header->data_tail;
>> +       *base = ((__u8 *)header) + pb->page_size;
>> +       *buf_size = pb->mmap_size;
>> +       return 0;
>> +}
>> +
>> +int perf_buffer__set_ring_buf_tail(const struct perf_buffer *pb, size_t buf_idx,
>> +                                  __u64 tail)
>> +{
>> +       struct perf_cpu_buf *cpu_buf;
>> +       struct perf_event_mmap_page *header;
>> +
>> +       if (buf_idx >= pb->cpu_cnt)
>> +               return libbpf_err(-EINVAL);
>> +
>> +       cpu_buf = pb->cpu_bufs[buf_idx];
>> +       if (!cpu_buf)
>> +               return libbpf_err(-ENOENT);
>> +
>> +       header = cpu_buf->base;
>> +       ring_buffer_write_tail(header, tail);
>> +       return 0;
>> +}
>> +
>>  int perf_buffer__epoll_fd(const struct perf_buffer *pb)
>>  {
>>         return pb->epoll_fd;
>> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
>> index 9e9a3fd3edd8..b6f6b6a12d70 100644
>> --- a/tools/lib/bpf/libbpf.h
>> +++ b/tools/lib/bpf/libbpf.h
>> @@ -1381,6 +1381,12 @@ LIBBPF_API int perf_buffer__consume(struct perf_buffer *pb);
>>  LIBBPF_API int perf_buffer__consume_buffer(struct perf_buffer *pb, size_t buf_idx);
>>  LIBBPF_API size_t perf_buffer__buffer_cnt(const struct perf_buffer *pb);
>>  LIBBPF_API int perf_buffer__buffer_fd(const struct perf_buffer *pb, size_t buf_idx);
>> +LIBBPF_API int perf_buffer__raw_ring_buf(const struct perf_buffer *pb,
>> +                                        size_t buf_idx, void **base,
>> +                                        size_t *buf_size, __u64 *head,
>> +                                        __u64 *tail);
>> +LIBBPF_API int perf_buffer__set_ring_buf_tail(const struct perf_buffer *pb,
>> +                                             size_t buf_idx, __u64 tail);
>>
>>  typedef enum bpf_perf_event_ret
>>         (*bpf_perf_event_print_t)(struct perf_event_header *hdr,
>> --
>> 2.36.1
>>