Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1270838

Re: [PATCH 07/13] bpf tools: Load a program with different instances using preprocessor

From "Wangnan (F)" <wangnan0@huawei.com>
Newsgroups linux.kernel
Subject Re: [PATCH 07/13] bpf tools: Load a program with different instances using preprocessor
Date 2015-11-17 05:00 +0100
Message-ID <qvFB0-86h-3@gated-at.bofh.it> (permalink)
References <qvrom-7n2-5@gated-at.bofh.it> <qvron-7n2-35@gated-at.bofh.it> <qvxk6-2PK-19@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw



On 2015/11/17 3:02, Arnaldo Carvalho de Melo wrote:
> Em Mon, Nov 16, 2015 at 12:10:09PM +0000, Wang Nan escreveu:
>> This patch is a preparation for BPF prologue support which allows
>> generating a series of BPF bytecode for fetching kernel data before
>> calling program code. With the newly introduced multiple instances
>> support, perf is able to create different prologues for different
>> kprobe points.
>>
>> Before this patch, a bpf_program can be loaded into kernel only once,
>> and get the only resuling fd. What this patch done is to allow creating
>> and loading different variants of one bpf_program, then fetching their
>> fds.
>>
>> Here describe the basic idea in this patch. The detail description of
>> the newly introduced APIs can be found in comment in the patch body.
>>
>> The key of this patch is the new mechanism in bpf_program__load().
>> Instead of loading BPF program into kernel directly, it calls a
>> 'pre-processor' to generate program instances which would be final
>> loaded into kernel based on the original code. To enable multiple
>> instances generation, libbpf passes an index to the pre-processor
>> so it know which instance is being loaded.
>>
>> Pre-processor should be passed from libbpf's user (perf) using
>> bpf_program__set_prep(). The number of instances and the relationship
>> between indics and the target instance should be clear when calling
>> bpf_program__set_prep().
>>
>> To retrive fd for a specific instance of a program,
>> bpf_program__nth_fd() is introduced. It return the resuling fd
>> according to index.
>>
>> Signed-off-by: Wang Nan <wangnan0@huawei.com>
>> Signed-off-by: He Kuang <hekuang@huawei.com>
>> Cc: Alexei Starovoitov <ast@kernel.org>
>> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
>> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> Cc: Zefan Li <lizefan@huawei.com>
>> Cc: pi3orama@163.com
>> ---
>>   tools/lib/bpf/libbpf.c | 145 ++++++++++++++++++++++++++++++++++++++++++++++---
>>   tools/lib/bpf/libbpf.h |  64 ++++++++++++++++++++++
>>   2 files changed, 200 insertions(+), 9 deletions(-)
>>
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index e176bad..fcfa39f 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -152,7 +152,11 @@ struct bpf_program {
>>   	} *reloc_desc;
>>   	int nr_reloc;
>>   
>> -	int fd;
>> +	struct {
>> +		int nr;
>> +		int *fds;
>> +	} instances;
>> +	bpf_program_prep_t preprocessor;
>>   
>>   	struct bpf_object *obj;
>>   	void *priv;
>> @@ -206,10 +210,24 @@ struct bpf_object {
>>   
>>   static void bpf_program__unload(struct bpf_program *prog)
>>   {
>> +	int i;
>> +
>>   	if (!prog)
>>   		return;
>>   
>> -	zclose(prog->fd);
>> +	/*
>> +	 * If the object is opened but the program is never loaded,
>> +	 * it is possible that prog->instances.nr == -1.
>> +	 */
>> +	if (prog->instances.nr > 0) {
>> +		for (i = 0; i < prog->instances.nr; i++)
>> +			zclose(prog->instances.fds[i]);
>> +	} else if (prog->instances.nr != -1)
>> +		pr_warning("Internal error: instances.nr is %d\n",
>> +			   prog->instances.nr);
>> +
>
> Multi line if/else blocks should be under {}, like you did for the if
> part, but forgot to do for the else part.
>
> I'm fixing this up, but please try to be consistent about these details
> next time, more below.
>
>> +	prog->instances.nr = -1;
>> +	zfree(&prog->instances.fds);
>>   }
>>   
>>   static void bpf_program__exit(struct bpf_program *prog)
>> @@ -260,7 +278,8 @@ bpf_program__init(void *data, size_t size, char *name, int idx,
>>   	memcpy(prog->insns, data,
>>   	       prog->insns_cnt * sizeof(struct bpf_insn));
>>   	prog->idx = idx;
>> -	prog->fd = -1;
>> +	prog->instances.fds = NULL;
>> +	prog->instances.nr = -1;
>>   
>>   	return 0;
>>   errout:
>> @@ -860,13 +879,73 @@ static int
>>   bpf_program__load(struct bpf_program *prog,
>>   		  char *license, u32 kern_version)
>>   {
>> -	int err, fd;
>> +	int err = 0, fd, i;
>> +
>> +	if (prog->instances.nr < 0 || !prog->instances.fds) {
>> +		if (prog->preprocessor) {
>> +			pr_warning("Internal error: can't load program '%s'\n",
>> +				   prog->section_name);
>> +			return -LIBBPF_ERRNO__INTERNAL;
>> +		}
> Those errors I think should come with some prefix to provide more
> context about what kind of "internal error" is this, in some cases I am
> adding "BPF", but not in all, more to think about how to make this
> clearer.

In bpf_program__set_prep, the parameters are already checked, so
if prog->preprocessor is set (!= NULL), the conditions in
'if' statement should never be satisified. Therefore it must an
internal error (bug?) in libbpf.

All LIBBPF error code except LIBBPF_ERRNO__INTERNAL can be triggered
by user by some way (for example, providing an invalid object file).
Therefore, we need to tell them the reason of failure formally, let
error messages hint users how to fix their fault.
LIBBPF_ERRNO__INTERNAL is different. If it raise there must be a bug
in libbpf or perf (incorrectly use libbpf's API). In this case, we
have already provided pr_{debug,warning} to developers. In my opinion,
formally reporting is not require because perf's user don't need to
be told too much about the internal implementation (it causes confusion).
All they need to know is "there is a bug, let's report it with '-v' output".

Thank you.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 00/13] perf tools: bpf: Improve BPF program ability Wang Nan <wangnan0@huawei.com> - 2015-11-16 13:50 +0100
  [PATCH 03/13] perf tools: Allow BPF program attach to uprobe events Wang Nan <wangnan0@huawei.com> - 2015-11-16 13:50 +0100
    Re: [PATCH 03/13] perf tools: Allow BPF program attach to uprobe  events Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-11-16 15:20 +0100
    [tip:perf/core] perf bpf:   Allow BPF program attach to uprobe events tip-bot for Wang Nan <tipbot@zytor.com> - 2015-11-23 17:10 +0100
  [PATCH 10/13] perf tools: Add prologue for BPF programs for fetching arguments Wang Nan <wangnan0@huawei.com> - 2015-11-16 13:50 +0100
    [tip:perf/core] perf bpf:   Add prologue for BPF programs for fetching arguments tip-bot for He Kuang <tipbot@zytor.com> - 2015-11-23 17:10 +0100
  [PATCH 04/13] perf tools: Allow BPF program attach to modules Wang Nan <wangnan0@huawei.com> - 2015-11-16 13:50 +0100
    [tip:perf/core] perf bpf:   Allow attaching BPF programs to modules symbols tip-bot for Wang Nan <tipbot@zytor.com> - 2015-11-23 17:10 +0100
  [PATCH 07/13] bpf tools: Load a program with different instances using preprocessor Wang Nan <wangnan0@huawei.com> - 2015-11-16 13:50 +0100
    Re: [PATCH 07/13] bpf tools: Load a program with different instances  using preprocessor Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-11-16 20:10 +0100
      Re: [PATCH 07/13] bpf tools: Load a program with different instances  using preprocessor "Wangnan (F)" <wangnan0@huawei.com> - 2015-11-17 05:00 +0100
    [tip:perf/core] bpf tools:   Load a program with different instances using preprocessor tip-bot for Wang Nan <tipbot@zytor.com> - 2015-11-23 17:10 +0100
  [PATCH 13/13] perf tools: Use same BPF program if arguments are identical Wang Nan <wangnan0@huawei.com> - 2015-11-16 14:00 +0100
    Re: [PATCH 13/13] perf tools: Use same BPF program if arguments are  identical "Wangnan (F)" <wangnan0@huawei.com> - 2015-11-17 04:10 +0100
      Re: [PATCH 13/13] perf tools: Use same BPF program if arguments are  identical Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-11-17 14:20 +0100
  [PATCH 12/13] perf test: Test BPF prologue Wang Nan <wangnan0@huawei.com> - 2015-11-16 14:00 +0100
    Re: [PATCH 12/13] perf test: Test BPF prologue Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-11-17 02:30 +0100
      Re: [PATCH 12/13] perf test: Test BPF prologue "Wangnan (F)" <wangnan0@huawei.com> - 2015-11-17 02:40 +0100
      Re: [PATCH 12/13] perf test: Test BPF prologue "Wangnan (F)" <wangnan0@huawei.com> - 2015-11-17 05:40 +0100
        Re: [PATCH 12/13] perf test: Test BPF prologue Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-11-17 13:30 +0100
      [PATCH 0/5] perf tools: Improve BPF support Wang Nan <wangnan0@huawei.com> - 2015-11-17 09:40 +0100
        [PATCH 5/5] perf test: Mute test cases if verbose == 0 Wang Nan <wangnan0@huawei.com> - 2015-11-17 09:40 +0100
          Re: [PATCH 5/5] perf test: Mute test cases if verbose == 0 Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-11-17 14:20 +0100
          [tip:perf/core] perf test:   Mute test cases error messages if verbose == 0 tip-bot for Wang Nan <tipbot@zytor.com> - 2015-11-23 17:20 +0100
        [PATCH 2/5] perf tools: Use same BPF program if arguments are identical Wang Nan <wangnan0@huawei.com> - 2015-11-17 09:40 +0100
          [tip:perf/core] perf bpf:   Use same BPF program if arguments are identical tip-bot for Wang Nan <tipbot@zytor.com> - 2015-11-23 17:10 +0100
        [PATCH 4/5] perf test: Print result for each subtest for BPF Wang Nan <wangnan0@huawei.com> - 2015-11-17 09:40 +0100
          [tip:perf/core] perf test: Print result for each BPF subtest tip-bot for Wang Nan <tipbot@zytor.com> - 2015-11-23 17:20 +0100
        [PATCH 1/5] perf test: Fix 2 bugs in 'perf test BPF' Wang Nan <wangnan0@huawei.com> - 2015-11-17 09:40 +0100
          Re: [PATCH 1/5] perf test: Fix 2 bugs in 'perf test BPF' Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-11-17 14:00 +0100
            Re: [PATCH 1/5] perf test: Fix 2 bugs in 'perf test BPF' pi3orama <pi3orama@163.com> - 2015-11-17 14:10 +0100
              Re: [PATCH 1/5] perf test: Fix 2 bugs in 'perf test BPF' Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-11-17 14:40 +0100
          [tip:perf/core] perf test: Fix 'perf test BPF'   when it fails to find a suitable vmlinux tip-bot for Wang Nan <tipbot@zytor.com> - 2015-11-23 17:10 +0100
        [PATCH 3/5] perf test: Print result for each subtest for llvm Wang Nan <wangnan0@huawei.com> - 2015-11-17 09:40 +0100
          Re: [PATCH 3/5] perf test: Print result for each subtest for llvm Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-11-17 14:10 +0100
          [tip:perf/core] perf test: Print result for each LLVM subtest tip-bot for Wang Nan <tipbot@zytor.com> - 2015-11-23 17:20 +0100
      Re: [PATCH 12/13] perf test: Test BPF prologue "Wangnan (F)" <wangnan0@huawei.com> - 2015-11-17 09:50 +0100
      [PATCH] perf record: Support custom vmlinux path Wang Nan <wangnan0@huawei.com> - 2015-11-17 11:00 +0100
        Re: [PATCH] perf record: Support custom vmlinux path Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-11-17 13:40 +0100
          Re: [PATCH] perf record: Support custom vmlinux path Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-11-17 13:50 +0100
            Re: [PATCH] perf record: Support custom vmlinux path pi3orama <pi3orama@163.com> - 2015-11-17 13:50 +0100
            [PATCH 0/2] perf tools: Builtin options related improvements Wang Nan <wangnan0@huawei.com> - 2015-11-19 15:10 +0100
              [PATCH 2/2] perf record: Support custom vmlinux path Wang Nan <wangnan0@huawei.com> - 2015-11-19 15:10 +0100
              [PATCH 1/2] perf tools: Always give options even it not compiled Wang Nan <wangnan0@huawei.com> - 2015-11-19 15:10 +0100
                RE: [PATCH 1/2] perf tools: Always give options even it not compiled 平松雅巳 / HIRAMATU,MASAMI   <masami.hiramatsu.pt@hitachi.com> - 2015-11-20 12:00 +0100
                Re: [PATCH 1/2] perf tools: Always give options even it not compiled "Wangnan (F)" <wangnan0@huawei.com> - 2015-11-26 09:10 +0100
                Re: [PATCH 1/2] perf tools: Always give options even it not compiled "Wangnan (F)" <wangnan0@huawei.com> - 2015-11-26 10:10 +0100
    [tip:perf/core] perf test:   Test the BPF prologue adding infrastructure tip-bot for Wang Nan <tipbot@zytor.com> - 2015-11-23 17:10 +0100
  [PATCH 06/13] perf tools: Allow BPF program config probing options Wang Nan <wangnan0@huawei.com> - 2015-11-16 14:00 +0100
    [tip:perf/core] perf bpf:   Allow BPF program config probing options tip-bot for Wang Nan <tipbot@zytor.com> - 2015-11-23 17:10 +0100
  [PATCH 08/13] perf tools: Add BPF_PROLOGUE config options for further patches Wang Nan <wangnan0@huawei.com> - 2015-11-16 14:00 +0100
    [tip:perf/core] perf bpf:   Add BPF_PROLOGUE config options for further patches tip-bot for Wang Nan <tipbot@zytor.com> - 2015-11-23 17:10 +0100
  [PATCH 11/13] perf tools: Generate prologue for BPF programs Wang Nan <wangnan0@huawei.com> - 2015-11-16 14:00 +0100
    [tip:perf/core] perf bpf: Generate prologue for BPF programs tip-bot for Wang Nan <tipbot@zytor.com> - 2015-11-23 17:10 +0100
  [PATCH 02/13] perf probe: Clear probe_trace_event when add_probe_trace_event() fails Wang Nan <wangnan0@huawei.com> - 2015-11-16 14:00 +0100
  Re: [PATCH 00/13] perf tools: bpf: Improve BPF program ability Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-11-16 15:20 +0100

csiph-web