netdev - Re: [PATCH bpf-next v4 1/2] trace_helpers.c: Add helpers to poll multiple perf FDs for events

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180605151539.2fb16370@cakuba.netronome.com>
Date:   Tue, 5 Jun 2018 15:16:36 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Toke Høiland-Jørgensen <toke@...e.dk>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH bpf-next v4 1/2] trace_helpers.c: Add helpers to poll
 multiple perf FDs for events

On Tue, 05 Jun 2018 16:50:00 +0200, Toke Høiland-Jørgensen wrote:
> Add two new helper functions to trace_helpers that supports polling
> multiple perf file descriptors for events. These are used to the XDP
> perf_event_output example, which needs to work with one perf fd per CPU.
> 
> Signed-off-by: Toke Høiland-Jørgensen <toke@...e.dk>
> ---
>  tools/testing/selftests/bpf/trace_helpers.c |   47 ++++++++++++++++++++++++++-
>  tools/testing/selftests/bpf/trace_helpers.h |    4 ++
>  2 files changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c
> index 3868dcb63420..1e62d89f34cf 100644
> --- a/tools/testing/selftests/bpf/trace_helpers.c
> +++ b/tools/testing/selftests/bpf/trace_helpers.c
> @@ -88,7 +88,7 @@ static int page_size;
>  static int page_cnt = 8;
>  static struct perf_event_mmap_page *header;
>  
> -int perf_event_mmap(int fd)
> +int perf_event_mmap_header(int fd, struct perf_event_mmap_page **header)
>  {
>  	void *base;
>  	int mmap_size;
> @@ -102,10 +102,15 @@ int perf_event_mmap(int fd)
>  		return -1;
>  	}
>  
> -	header = base;
> +	*header = base;
>  	return 0;
>  }
>  
> +int perf_event_mmap(int fd)
> +{
> +	return perf_event_mmap_header(fd, &header);
> +}
> +
>  static int perf_event_poll(int fd)
>  {
>  	struct pollfd pfd = { .fd = fd, .events = POLLIN };
> @@ -163,3 +168,41 @@ int perf_event_poller(int fd, perf_event_print_fn output_fn)
>  
>  	return ret;
>  }
> +
> +int perf_event_poller_multi(int *fds, struct perf_event_mmap_page **headers,
> +			    int num_fds, perf_event_print_fn output_fn)
> +{
> +	enum bpf_perf_event_ret ret;
> +	struct pollfd *pfds;
> +	void *buf = NULL;
> +	size_t len = 0;
> +	int i;
> +
> +	pfds = malloc(sizeof(*pfds) * num_fds);
> +	if (!pfds)
> +		return -1;

nit: this is correct, but better use LIBBPF_PERF_EVENT_ERROR?  This
function is supposed to return enum bpf_perf_event_ret values I assume?
In case someone moves this code to libbpf later on...

> +	memset(pfds, 0, sizeof(*pfds) * num_fds);
> +	for (i = 0; i < num_fds; i++) {
> +		pfds[i].fd = fds[i];
> +		pfds[i].events = POLLIN;
> +	}
> +
> +	for (;;) {
> +		poll(pfds, num_fds, 1000);
> +		for (i = 0; i < num_fds; i++) {
> +			if (pfds[i].revents) {

nit: save yourself a lever of indentation by doing:

if (!pfds[i].revents)
	continue;

and you won't have to go over 80 chars.

> +				ret = bpf_perf_event_read_simple(headers[i], page_cnt * page_size,
> +								page_size, &buf, &len,
> +								bpf_perf_event_print,
> +								output_fn);
> +				if (ret != LIBBPF_PERF_EVENT_CONT)
> +					break;
> +			}
> +		}
> +	}
> +	free(buf);
> +	free(pfds);
> +
> +	return ret;
> +}