diff mbox series

[v5,perf,bpf,07/15] perf, bpf: save bpf_prog_info information as headers to perf.data

Message ID 20190228050643.958685-8-songliubraving@fb.com
State Changes Requested
Delegated to: BPF Maintainers
Headers show
Series perf annotation of BPF programs | expand

Commit Message

Song Liu Feb. 28, 2019, 5:06 a.m. UTC
This patch enables perf-record to save bpf_prog_info information as
headers to perf.data. A new header type HEADER_BPF_PROG_INFO is
introduced for this data.

Signed-off-by: Song Liu <songliubraving@fb.com>
---
 tools/perf/util/header.c | 143 ++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/header.h |   1 +
 2 files changed, 143 insertions(+), 1 deletion(-)

Comments

Jiri Olsa March 4, 2019, 1:52 p.m. UTC | #1
On Wed, Feb 27, 2019 at 09:06:35PM -0800, Song Liu wrote:

SNIP

> +static int process_bpf_prog_info(struct feat_fd *ff,
> +				 void *data __maybe_unused)
> +{
> +	struct bpf_prog_info_linear *info_linear;
> +	struct bpf_prog_info_node *info_node;
> +	struct perf_env *env = &ff->ph->env;
> +	u32 count, i;
> +	int err = -1;
> +
> +	if (do_read_u32(ff, &count))
> +		return -1;
> +
> +	down_write(&env->bpf_progs.lock);
> +
> +	for (i = 0; i < count; ++i) {
> +		u32 info_len, data_len;
> +
> +		info_linear = NULL;
> +		info_node = NULL;
> +		if (do_read_u32(ff, &info_len))
> +			goto out;
> +		if (do_read_u32(ff, &data_len))
> +			goto out;
> +
> +		if (info_len > sizeof(struct bpf_prog_info)) {
> +			pr_warning("detected invalid bpf_prog_info\n");
> +			goto out;
> +		}
> +
> +		info_linear = malloc(sizeof(struct bpf_prog_info_linear) +
> +				     data_len);
> +		if (!info_linear)
> +			goto out;
> +		info_linear->info_len = sizeof(struct bpf_prog_info);
> +		info_linear->data_len = data_len;
> +		if (do_read_u64(ff, (u64 *)(&info_linear->arrays)))
> +			goto out;
> +		if (__do_read(ff, &info_linear->info, info_len))
> +			goto out;
> +		if (info_len < sizeof(struct bpf_prog_info))
> +			memset(((void *)(&info_linear->info)) + info_len, 0,
> +			       sizeof(struct bpf_prog_info) - info_len);
> +
> +		if (__do_read(ff, info_linear->data, data_len))
> +			goto out;
> +
> +		/* endian mismatch, drop the info, continue */
> +		if (ff->ph->needs_swap) {
> +			free(info_linear);
> +			continue;
> +		}

so in this case we can check for needs_swap in the begining
of the function and bail out without reading all the data

also please display soem error message saying we don't support
ebpf progs data report over the different endianity

jirka
Jiri Olsa March 4, 2019, 1:52 p.m. UTC | #2
On Wed, Feb 27, 2019 at 09:06:35PM -0800, Song Liu wrote:

SNIP

> +static int process_bpf_prog_info(struct feat_fd *ff,
> +				 void *data __maybe_unused)
> +{
> +	struct bpf_prog_info_linear *info_linear;
> +	struct bpf_prog_info_node *info_node;
> +	struct perf_env *env = &ff->ph->env;
> +	u32 count, i;
> +	int err = -1;
> +
> +	if (do_read_u32(ff, &count))
> +		return -1;
> +
> +	down_write(&env->bpf_progs.lock);
> +
> +	for (i = 0; i < count; ++i) {
> +		u32 info_len, data_len;
> +
> +		info_linear = NULL;
> +		info_node = NULL;
> +		if (do_read_u32(ff, &info_len))
> +			goto out;
> +		if (do_read_u32(ff, &data_len))
> +			goto out;
> +
> +		if (info_len > sizeof(struct bpf_prog_info)) {
> +			pr_warning("detected invalid bpf_prog_info\n");
> +			goto out;
> +		}
> +
> +		info_linear = malloc(sizeof(struct bpf_prog_info_linear) +
> +				     data_len);
> +		if (!info_linear)
> +			goto out;
> +		info_linear->info_len = sizeof(struct bpf_prog_info);
> +		info_linear->data_len = data_len;
> +		if (do_read_u64(ff, (u64 *)(&info_linear->arrays)))
> +			goto out;
> +		if (__do_read(ff, &info_linear->info, info_len))
> +			goto out;
> +		if (info_len < sizeof(struct bpf_prog_info))
> +			memset(((void *)(&info_linear->info)) + info_len, 0,
> +			       sizeof(struct bpf_prog_info) - info_len);
> +
> +		if (__do_read(ff, info_linear->data, data_len))
> +			goto out;
> +
> +		/* endian mismatch, drop the info, continue */
> +		if (ff->ph->needs_swap) {
> +			free(info_linear);
> +			continue;
> +		}

same here, please check this on the beginning
and add a warning message

thanks,
jrika

> +
> +		info_node = malloc(sizeof(struct bpf_prog_info_node));
> +		if (!info_node)
> +			goto out;
> +
> +		/* after reading from file, translate offset to address */
> +		bpf_program__bpil_offs_to_addr(info_linear);
> +		info_node->info_linear = info_linear;
> +		perf_env__insert_bpf_prog_info(env, info_node);
> +	}

SNIP
Jiri Olsa March 4, 2019, 1:52 p.m. UTC | #3
On Wed, Feb 27, 2019 at 09:06:35PM -0800, Song Liu wrote:
> This patch enables perf-record to save bpf_prog_info information as
> headers to perf.data. A new header type HEADER_BPF_PROG_INFO is
> introduced for this data.
> 
> Signed-off-by: Song Liu <songliubraving@fb.com>
> ---
>  tools/perf/util/header.c | 143 ++++++++++++++++++++++++++++++++++++++-
>  tools/perf/util/header.h |   1 +
>  2 files changed, 143 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 4b88de5e9192..16f5bedb0b7d 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -18,6 +18,7 @@
>  #include <sys/utsname.h>
>  #include <linux/time64.h>
>  #include <dirent.h>
> +#include <bpf/libbpf.h>
>  
>  #include "evlist.h"
>  #include "evsel.h"
> @@ -39,6 +40,7 @@
>  #include "tool.h"
>  #include "time-utils.h"
>  #include "units.h"
> +#include "bpf-event.h"
>  
>  #include "sane_ctype.h"
>  
> @@ -1074,6 +1076,51 @@ static int write_clockid(struct feat_fd *ff,
>  			sizeof(ff->ph->env.clockid_res_ns));
>  }
>  
> +static int write_bpf_prog_info(struct feat_fd *ff,
> +			       struct perf_evlist *evlist __maybe_unused)
> +{
> +	struct perf_env *env = &ff->ph->env;
> +	struct rb_root *root;
> +	struct rb_node *next;
> +	u32 count = 0;
> +	int ret;
> +
> +	down_read(&env->bpf_progs.lock);
> +
> +	root = &env->bpf_progs.infos;
> +	next = rb_first(root);
> +	while (next) {
> +		++count;
> +		next = rb_next(next);
> +	}
> +
> +	ret = do_write(ff, &count, sizeof(count));
> +
> +	if (ret < 0)
> +		goto out;
> +
> +	next = rb_first(root);
> +	while (next) {
> +		struct bpf_prog_info_node *node;
> +		size_t len;
> +
> +		node = rb_entry(next, struct bpf_prog_info_node, rb_node);
> +		next = rb_next(&node->rb_node);
> +		len = sizeof(struct bpf_prog_info_linear) +
> +			node->info_linear->data_len;
> +
> +		/* before writing to file, translate address to offset */
> +		bpf_program__bpil_addr_to_offs(node->info_linear);
> +		ret = do_write(ff, node->info_linear, len);
> +		bpf_program__bpil_offs_to_addr(node->info_linear);

what's the reason to call this before the error check?

thanks,
jirka

> +		if (ret < 0)
> +			goto out;
> +	}
> +out:
> +	up_read(&env->bpf_progs.lock);
> +	return ret;
> +}

SNIP
Jiri Olsa March 4, 2019, 1:52 p.m. UTC | #4
On Wed, Feb 27, 2019 at 09:06:35PM -0800, Song Liu wrote:
> This patch enables perf-record to save bpf_prog_info information as
> headers to perf.data. A new header type HEADER_BPF_PROG_INFO is
> introduced for this data.
> 
> Signed-off-by: Song Liu <songliubraving@fb.com>
> ---
>  tools/perf/util/header.c | 143 ++++++++++++++++++++++++++++++++++++++-
>  tools/perf/util/header.h |   1 +
>  2 files changed, 143 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 4b88de5e9192..16f5bedb0b7d 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -18,6 +18,7 @@
>  #include <sys/utsname.h>
>  #include <linux/time64.h>
>  #include <dirent.h>
> +#include <bpf/libbpf.h>
>  
>  #include "evlist.h"
>  #include "evsel.h"
> @@ -39,6 +40,7 @@
>  #include "tool.h"
>  #include "time-utils.h"
>  #include "units.h"
> +#include "bpf-event.h"
>  
>  #include "sane_ctype.h"
>  
> @@ -1074,6 +1076,51 @@ static int write_clockid(struct feat_fd *ff,
>  			sizeof(ff->ph->env.clockid_res_ns));
>  }
>  
> +static int write_bpf_prog_info(struct feat_fd *ff,
> +			       struct perf_evlist *evlist __maybe_unused)
> +{
> +	struct perf_env *env = &ff->ph->env;
> +	struct rb_root *root;
> +	struct rb_node *next;
> +	u32 count = 0;
> +	int ret;
> +
> +	down_read(&env->bpf_progs.lock);
> +
> +	root = &env->bpf_progs.infos;
> +	next = rb_first(root);
> +	while (next) {
> +		++count;
> +		next = rb_next(next);
> +	}
> +
> +	ret = do_write(ff, &count, sizeof(count));
> +

extra new line

> +	if (ret < 0)
> +		goto out;
> +
> +	next = rb_first(root);
> +	while (next) {
> +		struct bpf_prog_info_node *node;
> +		size_t len;
> +
> +		node = rb_entry(next, struct bpf_prog_info_node, rb_node);
> +		next = rb_next(&node->rb_node);
> +		len = sizeof(struct bpf_prog_info_linear) +
> +			node->info_linear->data_len;
> +
> +		/* before writing to file, translate address to offset */
> +		bpf_program__bpil_addr_to_offs(node->info_linear);
> +		ret = do_write(ff, node->info_linear, len);
> +		bpf_program__bpil_offs_to_addr(node->info_linear);
> +		if (ret < 0)
> +			goto out;
> +	}
> +out:
> +	up_read(&env->bpf_progs.lock);
> +	return ret;
> +}

SNIP
Song Liu March 4, 2019, 7:36 p.m. UTC | #5
> On Mar 4, 2019, at 5:52 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> 
> On Wed, Feb 27, 2019 at 09:06:35PM -0800, Song Liu wrote:
> 
> SNIP
> 
>> +static int process_bpf_prog_info(struct feat_fd *ff,
>> +				 void *data __maybe_unused)
>> +{
>> +	struct bpf_prog_info_linear *info_linear;
>> +	struct bpf_prog_info_node *info_node;
>> +	struct perf_env *env = &ff->ph->env;
>> +	u32 count, i;
>> +	int err = -1;
>> +
>> +	if (do_read_u32(ff, &count))
>> +		return -1;
>> +
>> +	down_write(&env->bpf_progs.lock);
>> +
>> +	for (i = 0; i < count; ++i) {
>> +		u32 info_len, data_len;
>> +
>> +		info_linear = NULL;
>> +		info_node = NULL;
>> +		if (do_read_u32(ff, &info_len))
>> +			goto out;
>> +		if (do_read_u32(ff, &data_len))
>> +			goto out;
>> +
>> +		if (info_len > sizeof(struct bpf_prog_info)) {
>> +			pr_warning("detected invalid bpf_prog_info\n");
>> +			goto out;
>> +		}
>> +
>> +		info_linear = malloc(sizeof(struct bpf_prog_info_linear) +
>> +				     data_len);
>> +		if (!info_linear)
>> +			goto out;
>> +		info_linear->info_len = sizeof(struct bpf_prog_info);
>> +		info_linear->data_len = data_len;
>> +		if (do_read_u64(ff, (u64 *)(&info_linear->arrays)))
>> +			goto out;
>> +		if (__do_read(ff, &info_linear->info, info_len))
>> +			goto out;
>> +		if (info_len < sizeof(struct bpf_prog_info))
>> +			memset(((void *)(&info_linear->info)) + info_len, 0,
>> +			       sizeof(struct bpf_prog_info) - info_len);
>> +
>> +		if (__do_read(ff, info_linear->data, data_len))
>> +			goto out;
>> +
>> +		/* endian mismatch, drop the info, continue */
>> +		if (ff->ph->needs_swap) {
>> +			free(info_linear);
>> +			continue;
>> +		}
> 
> so in this case we can check for needs_swap in the begining
> of the function and bail out without reading all the data

If we bail out, perf-record will fail. If we read all the data 
but ignore them, perf-record will continue without bpf annotation 
support. I think the latter is a better experience with little 
effort here. 

> 
> also please display soem error message saying we don't support
> ebpf progs data report over the different endianity

I will add a warning in the next version. 

Thanks,
Song
Song Liu March 4, 2019, 7:41 p.m. UTC | #6
> On Mar 4, 2019, at 5:52 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> 
> On Wed, Feb 27, 2019 at 09:06:35PM -0800, Song Liu wrote:
>> This patch enables perf-record to save bpf_prog_info information as
>> headers to perf.data. A new header type HEADER_BPF_PROG_INFO is
>> introduced for this data.
>> 
>> Signed-off-by: Song Liu <songliubraving@fb.com>
>> ---
>> tools/perf/util/header.c | 143 ++++++++++++++++++++++++++++++++++++++-
>> tools/perf/util/header.h |   1 +
>> 2 files changed, 143 insertions(+), 1 deletion(-)
>> 
>> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
>> index 4b88de5e9192..16f5bedb0b7d 100644
>> --- a/tools/perf/util/header.c
>> +++ b/tools/perf/util/header.c
>> @@ -18,6 +18,7 @@
>> #include <sys/utsname.h>
>> #include <linux/time64.h>
>> #include <dirent.h>
>> +#include <bpf/libbpf.h>
>> 
>> #include "evlist.h"
>> #include "evsel.h"
>> @@ -39,6 +40,7 @@
>> #include "tool.h"
>> #include "time-utils.h"
>> #include "units.h"
>> +#include "bpf-event.h"
>> 
>> #include "sane_ctype.h"
>> 
>> @@ -1074,6 +1076,51 @@ static int write_clockid(struct feat_fd *ff,
>> 			sizeof(ff->ph->env.clockid_res_ns));
>> }
>> 
>> +static int write_bpf_prog_info(struct feat_fd *ff,
>> +			       struct perf_evlist *evlist __maybe_unused)
>> +{
>> +	struct perf_env *env = &ff->ph->env;
>> +	struct rb_root *root;
>> +	struct rb_node *next;
>> +	u32 count = 0;
>> +	int ret;
>> +
>> +	down_read(&env->bpf_progs.lock);
>> +
>> +	root = &env->bpf_progs.infos;
>> +	next = rb_first(root);
>> +	while (next) {
>> +		++count;
>> +		next = rb_next(next);
>> +	}
>> +
>> +	ret = do_write(ff, &count, sizeof(count));
>> +
>> +	if (ret < 0)
>> +		goto out;
>> +
>> +	next = rb_first(root);
>> +	while (next) {
>> +		struct bpf_prog_info_node *node;
>> +		size_t len;
>> +
>> +		node = rb_entry(next, struct bpf_prog_info_node, rb_node);
>> +		next = rb_next(&node->rb_node);
>> +		len = sizeof(struct bpf_prog_info_linear) +
>> +			node->info_linear->data_len;
>> +
>> +		/* before writing to file, translate address to offset */
>> +		bpf_program__bpil_addr_to_offs(node->info_linear);
>> +		ret = do_write(ff, node->info_linear, len);
>> +		bpf_program__bpil_offs_to_addr(node->info_linear);
> 
> what's the reason to call this before the error check?

This will translate the all the pointers to the right address 
(instead of offsets). We only need the offsets when writing to 
files. With this approach, the data is not changed whether the
function succeeds or not. It is probably not necessary right 
now. But I think this may save us some debugging efforts in 
the future. 

Thanks,
Song

> 
> thanks,
> jirka
> 
>> +		if (ret < 0)
>> +			goto out;
>> +	}
>> +out:
>> +	up_read(&env->bpf_progs.lock);
>> +	return ret;
>> +}
> 
> SNIP
Jiri Olsa March 4, 2019, 8:23 p.m. UTC | #7
On Mon, Mar 04, 2019 at 07:36:14PM +0000, Song Liu wrote:
> 
> 
> > On Mar 4, 2019, at 5:52 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> > 
> > On Wed, Feb 27, 2019 at 09:06:35PM -0800, Song Liu wrote:
> > 
> > SNIP
> > 
> >> +static int process_bpf_prog_info(struct feat_fd *ff,
> >> +				 void *data __maybe_unused)
> >> +{
> >> +	struct bpf_prog_info_linear *info_linear;
> >> +	struct bpf_prog_info_node *info_node;
> >> +	struct perf_env *env = &ff->ph->env;
> >> +	u32 count, i;
> >> +	int err = -1;
> >> +
> >> +	if (do_read_u32(ff, &count))
> >> +		return -1;
> >> +
> >> +	down_write(&env->bpf_progs.lock);
> >> +
> >> +	for (i = 0; i < count; ++i) {
> >> +		u32 info_len, data_len;
> >> +
> >> +		info_linear = NULL;
> >> +		info_node = NULL;
> >> +		if (do_read_u32(ff, &info_len))
> >> +			goto out;
> >> +		if (do_read_u32(ff, &data_len))
> >> +			goto out;
> >> +
> >> +		if (info_len > sizeof(struct bpf_prog_info)) {
> >> +			pr_warning("detected invalid bpf_prog_info\n");
> >> +			goto out;
> >> +		}
> >> +
> >> +		info_linear = malloc(sizeof(struct bpf_prog_info_linear) +
> >> +				     data_len);
> >> +		if (!info_linear)
> >> +			goto out;
> >> +		info_linear->info_len = sizeof(struct bpf_prog_info);
> >> +		info_linear->data_len = data_len;
> >> +		if (do_read_u64(ff, (u64 *)(&info_linear->arrays)))
> >> +			goto out;
> >> +		if (__do_read(ff, &info_linear->info, info_len))
> >> +			goto out;
> >> +		if (info_len < sizeof(struct bpf_prog_info))
> >> +			memset(((void *)(&info_linear->info)) + info_len, 0,
> >> +			       sizeof(struct bpf_prog_info) - info_len);
> >> +
> >> +		if (__do_read(ff, info_linear->data, data_len))
> >> +			goto out;
> >> +
> >> +		/* endian mismatch, drop the info, continue */
> >> +		if (ff->ph->needs_swap) {
> >> +			free(info_linear);
> >> +			continue;
> >> +		}
> > 
> > so in this case we can check for needs_swap in the begining
> > of the function and bail out without reading all the data
> 
> If we bail out, perf-record will fail. If we read all the data 
> but ignore them, perf-record will continue without bpf annotation 
> support. I think the latter is a better experience with little 
> effort here. 

i did not mean bail out with error, just return 0, warn and go on

jirka

> 
> > 
> > also please display soem error message saying we don't support
> > ebpf progs data report over the different endianity
> 
> I will add a warning in the next version. 
> 
> Thanks,
> Song
>
Song Liu March 4, 2019, 8:31 p.m. UTC | #8
> On Mar 4, 2019, at 12:23 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> 
> On Mon, Mar 04, 2019 at 07:36:14PM +0000, Song Liu wrote:
>> 
>> 
>>> On Mar 4, 2019, at 5:52 AM, Jiri Olsa <jolsa@redhat.com> wrote:
>>> 
>>> On Wed, Feb 27, 2019 at 09:06:35PM -0800, Song Liu wrote:
>>> 
>>> SNIP
>>> 
>>>> +static int process_bpf_prog_info(struct feat_fd *ff,
>>>> +				 void *data __maybe_unused)
>>>> +{
>>>> +	struct bpf_prog_info_linear *info_linear;
>>>> +	struct bpf_prog_info_node *info_node;
>>>> +	struct perf_env *env = &ff->ph->env;
>>>> +	u32 count, i;
>>>> +	int err = -1;
>>>> +
>>>> +	if (do_read_u32(ff, &count))
>>>> +		return -1;
>>>> +
>>>> +	down_write(&env->bpf_progs.lock);
>>>> +
>>>> +	for (i = 0; i < count; ++i) {
>>>> +		u32 info_len, data_len;
>>>> +
>>>> +		info_linear = NULL;
>>>> +		info_node = NULL;
>>>> +		if (do_read_u32(ff, &info_len))
>>>> +			goto out;
>>>> +		if (do_read_u32(ff, &data_len))
>>>> +			goto out;
>>>> +
>>>> +		if (info_len > sizeof(struct bpf_prog_info)) {
>>>> +			pr_warning("detected invalid bpf_prog_info\n");
>>>> +			goto out;
>>>> +		}
>>>> +
>>>> +		info_linear = malloc(sizeof(struct bpf_prog_info_linear) +
>>>> +				     data_len);
>>>> +		if (!info_linear)
>>>> +			goto out;
>>>> +		info_linear->info_len = sizeof(struct bpf_prog_info);
>>>> +		info_linear->data_len = data_len;
>>>> +		if (do_read_u64(ff, (u64 *)(&info_linear->arrays)))
>>>> +			goto out;
>>>> +		if (__do_read(ff, &info_linear->info, info_len))
>>>> +			goto out;
>>>> +		if (info_len < sizeof(struct bpf_prog_info))
>>>> +			memset(((void *)(&info_linear->info)) + info_len, 0,
>>>> +			       sizeof(struct bpf_prog_info) - info_len);
>>>> +
>>>> +		if (__do_read(ff, info_linear->data, data_len))
>>>> +			goto out;
>>>> +
>>>> +		/* endian mismatch, drop the info, continue */
>>>> +		if (ff->ph->needs_swap) {
>>>> +			free(info_linear);
>>>> +			continue;
>>>> +		}
>>> 
>>> so in this case we can check for needs_swap in the begining
>>> of the function and bail out without reading all the data
>> 
>> If we bail out, perf-record will fail. If we read all the data 
>> but ignore them, perf-record will continue without bpf annotation 
>> support. I think the latter is a better experience with little 
>> effort here. 
> 
> i did not mean bail out with error, just return 0, warn and go on
> 
> jirka

In this case, do we need to read and discard all the data? If not, 
the next header type (or data) will interpret these data as 
something else, right? Or did I miss some protection against such
cases?

Thanks,
Song

> 
>> 
>>> 
>>> also please display soem error message saying we don't support
>>> ebpf progs data report over the different endianity
>> 
>> I will add a warning in the next version. 
>> 
>> Thanks,
>> Song
>>
Jiri Olsa March 4, 2019, 8:37 p.m. UTC | #9
On Mon, Mar 04, 2019 at 07:41:10PM +0000, Song Liu wrote:
> 
> 
> > On Mar 4, 2019, at 5:52 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> > 
> > On Wed, Feb 27, 2019 at 09:06:35PM -0800, Song Liu wrote:
> >> This patch enables perf-record to save bpf_prog_info information as
> >> headers to perf.data. A new header type HEADER_BPF_PROG_INFO is
> >> introduced for this data.
> >> 
> >> Signed-off-by: Song Liu <songliubraving@fb.com>
> >> ---
> >> tools/perf/util/header.c | 143 ++++++++++++++++++++++++++++++++++++++-
> >> tools/perf/util/header.h |   1 +
> >> 2 files changed, 143 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> >> index 4b88de5e9192..16f5bedb0b7d 100644
> >> --- a/tools/perf/util/header.c
> >> +++ b/tools/perf/util/header.c
> >> @@ -18,6 +18,7 @@
> >> #include <sys/utsname.h>
> >> #include <linux/time64.h>
> >> #include <dirent.h>
> >> +#include <bpf/libbpf.h>
> >> 
> >> #include "evlist.h"
> >> #include "evsel.h"
> >> @@ -39,6 +40,7 @@
> >> #include "tool.h"
> >> #include "time-utils.h"
> >> #include "units.h"
> >> +#include "bpf-event.h"
> >> 
> >> #include "sane_ctype.h"
> >> 
> >> @@ -1074,6 +1076,51 @@ static int write_clockid(struct feat_fd *ff,
> >> 			sizeof(ff->ph->env.clockid_res_ns));
> >> }
> >> 
> >> +static int write_bpf_prog_info(struct feat_fd *ff,
> >> +			       struct perf_evlist *evlist __maybe_unused)
> >> +{
> >> +	struct perf_env *env = &ff->ph->env;
> >> +	struct rb_root *root;
> >> +	struct rb_node *next;
> >> +	u32 count = 0;
> >> +	int ret;
> >> +
> >> +	down_read(&env->bpf_progs.lock);
> >> +
> >> +	root = &env->bpf_progs.infos;
> >> +	next = rb_first(root);
> >> +	while (next) {
> >> +		++count;
> >> +		next = rb_next(next);
> >> +	}
> >> +
> >> +	ret = do_write(ff, &count, sizeof(count));
> >> +
> >> +	if (ret < 0)
> >> +		goto out;
> >> +
> >> +	next = rb_first(root);
> >> +	while (next) {
> >> +		struct bpf_prog_info_node *node;
> >> +		size_t len;
> >> +
> >> +		node = rb_entry(next, struct bpf_prog_info_node, rb_node);
> >> +		next = rb_next(&node->rb_node);
> >> +		len = sizeof(struct bpf_prog_info_linear) +
> >> +			node->info_linear->data_len;
> >> +
> >> +		/* before writing to file, translate address to offset */
> >> +		bpf_program__bpil_addr_to_offs(node->info_linear);
> >> +		ret = do_write(ff, node->info_linear, len);
> >> +		bpf_program__bpil_offs_to_addr(node->info_linear);
> > 
> > what's the reason to call this before the error check?
> 
> This will translate the all the pointers to the right address 
> (instead of offsets). We only need the offsets when writing to 
> files. With this approach, the data is not changed whether the
> function succeeds or not. It is probably not necessary right 
> now. But I think this may save us some debugging efforts in 
> the future. 

ok, please make comment with this

jirka
diff mbox series

Patch

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 4b88de5e9192..16f5bedb0b7d 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -18,6 +18,7 @@ 
 #include <sys/utsname.h>
 #include <linux/time64.h>
 #include <dirent.h>
+#include <bpf/libbpf.h>
 
 #include "evlist.h"
 #include "evsel.h"
@@ -39,6 +40,7 @@ 
 #include "tool.h"
 #include "time-utils.h"
 #include "units.h"
+#include "bpf-event.h"
 
 #include "sane_ctype.h"
 
@@ -1074,6 +1076,51 @@  static int write_clockid(struct feat_fd *ff,
 			sizeof(ff->ph->env.clockid_res_ns));
 }
 
+static int write_bpf_prog_info(struct feat_fd *ff,
+			       struct perf_evlist *evlist __maybe_unused)
+{
+	struct perf_env *env = &ff->ph->env;
+	struct rb_root *root;
+	struct rb_node *next;
+	u32 count = 0;
+	int ret;
+
+	down_read(&env->bpf_progs.lock);
+
+	root = &env->bpf_progs.infos;
+	next = rb_first(root);
+	while (next) {
+		++count;
+		next = rb_next(next);
+	}
+
+	ret = do_write(ff, &count, sizeof(count));
+
+	if (ret < 0)
+		goto out;
+
+	next = rb_first(root);
+	while (next) {
+		struct bpf_prog_info_node *node;
+		size_t len;
+
+		node = rb_entry(next, struct bpf_prog_info_node, rb_node);
+		next = rb_next(&node->rb_node);
+		len = sizeof(struct bpf_prog_info_linear) +
+			node->info_linear->data_len;
+
+		/* before writing to file, translate address to offset */
+		bpf_program__bpil_addr_to_offs(node->info_linear);
+		ret = do_write(ff, node->info_linear, len);
+		bpf_program__bpil_offs_to_addr(node->info_linear);
+		if (ret < 0)
+			goto out;
+	}
+out:
+	up_read(&env->bpf_progs.lock);
+	return ret;
+}
+
 static int cpu_cache_level__sort(const void *a, const void *b)
 {
 	struct cpu_cache_level *cache_a = (struct cpu_cache_level *)a;
@@ -1554,6 +1601,29 @@  static void print_clockid(struct feat_fd *ff, FILE *fp)
 		ff->ph->env.clockid_res_ns * 1000);
 }
 
+static void print_bpf_prog_info(struct feat_fd *ff, FILE *fp)
+{
+	struct perf_env *env = &ff->ph->env;
+	struct rb_root *root;
+	struct rb_node *next;
+
+	down_read(&env->bpf_progs.lock);
+
+	root = &env->bpf_progs.infos;
+	next = rb_first(root);
+
+	while (next) {
+		struct bpf_prog_info_node *node;
+
+		node = rb_entry(next, struct bpf_prog_info_node, rb_node);
+		next = rb_next(&node->rb_node);
+		fprintf(fp, "# bpf_prog_info of id %u\n",
+			node->info_linear->info.id);
+	}
+
+	up_read(&env->bpf_progs.lock);
+}
+
 static void free_event_desc(struct perf_evsel *events)
 {
 	struct perf_evsel *evsel;
@@ -2586,6 +2656,76 @@  static int process_clockid(struct feat_fd *ff,
 	return 0;
 }
 
+static int process_bpf_prog_info(struct feat_fd *ff,
+				 void *data __maybe_unused)
+{
+	struct bpf_prog_info_linear *info_linear;
+	struct bpf_prog_info_node *info_node;
+	struct perf_env *env = &ff->ph->env;
+	u32 count, i;
+	int err = -1;
+
+	if (do_read_u32(ff, &count))
+		return -1;
+
+	down_write(&env->bpf_progs.lock);
+
+	for (i = 0; i < count; ++i) {
+		u32 info_len, data_len;
+
+		info_linear = NULL;
+		info_node = NULL;
+		if (do_read_u32(ff, &info_len))
+			goto out;
+		if (do_read_u32(ff, &data_len))
+			goto out;
+
+		if (info_len > sizeof(struct bpf_prog_info)) {
+			pr_warning("detected invalid bpf_prog_info\n");
+			goto out;
+		}
+
+		info_linear = malloc(sizeof(struct bpf_prog_info_linear) +
+				     data_len);
+		if (!info_linear)
+			goto out;
+		info_linear->info_len = sizeof(struct bpf_prog_info);
+		info_linear->data_len = data_len;
+		if (do_read_u64(ff, (u64 *)(&info_linear->arrays)))
+			goto out;
+		if (__do_read(ff, &info_linear->info, info_len))
+			goto out;
+		if (info_len < sizeof(struct bpf_prog_info))
+			memset(((void *)(&info_linear->info)) + info_len, 0,
+			       sizeof(struct bpf_prog_info) - info_len);
+
+		if (__do_read(ff, info_linear->data, data_len))
+			goto out;
+
+		/* endian mismatch, drop the info, continue */
+		if (ff->ph->needs_swap) {
+			free(info_linear);
+			continue;
+		}
+
+		info_node = malloc(sizeof(struct bpf_prog_info_node));
+		if (!info_node)
+			goto out;
+
+		/* after reading from file, translate offset to address */
+		bpf_program__bpil_offs_to_addr(info_linear);
+		info_node->info_linear = info_linear;
+		perf_env__insert_bpf_prog_info(env, info_node);
+	}
+
+	return 0;
+out:
+	free(info_linear);
+	free(info_node);
+	up_write(&env->bpf_progs.lock);
+	return err;
+}
+
 struct feature_ops {
 	int (*write)(struct feat_fd *ff, struct perf_evlist *evlist);
 	void (*print)(struct feat_fd *ff, FILE *fp);
@@ -2645,7 +2785,8 @@  static const struct feature_ops feat_ops[HEADER_LAST_FEATURE] = {
 	FEAT_OPN(CACHE,		cache,		true),
 	FEAT_OPR(SAMPLE_TIME,	sample_time,	false),
 	FEAT_OPR(MEM_TOPOLOGY,	mem_topology,	true),
-	FEAT_OPR(CLOCKID,       clockid,        false)
+	FEAT_OPR(CLOCKID,       clockid,        false),
+	FEAT_OPR(BPF_PROG_INFO, bpf_prog_info,  false)
 };
 
 struct header_print_data {
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 0d553ddca0a3..0785c91b4c3a 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -39,6 +39,7 @@  enum {
 	HEADER_SAMPLE_TIME,
 	HEADER_MEM_TOPOLOGY,
 	HEADER_CLOCKID,
+	HEADER_BPF_PROG_INFO,
 	HEADER_LAST_FEATURE,
 	HEADER_FEAT_BITS	= 256,
 };