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

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Wed, Oct 14, 2015 at 12:41:24PM +0000, Wang Nan escreveu:
> In this patch, caller of libbpf is able to control the loaded programs
> by installing a preprocessor callback for a BPF program. With
> preprocessor, different instances can be created from one BPF program.

Why would one want to do that? I'm new to eBPF, as many other here, I
guess, so giving some more context as to what will this achieve will
help me in processing these patches.
 
> This patch will be used by perf to generate different prologue for
> different 'struct probe_trace_event' instances matched by one
> 'struct perf_probe_event'.
> 
> bpf_program__set_prep() is added to support this feature. Caller
> should pass libbpf the number of instances should be created and a
                                          that
> preprocessor function which will be called when doing real loading.
                                                      the
> The callback should return instructions arrays for each instances.
                                instruction arrays for each instance.
> 
> fd field in bpf_programs is replaced by instance, which has an nr field

The fd field in the bpf_programs struct is replaced by instance
(instance of what?)

> and fds array. bpf_program__nth_fd() is introduced for read fd of
> instances. Old interface bpf_program__fd() is reimplemented by
> returning the first fd.
> 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Signed-off-by: He Kuang <hekuang@huawei.com>
> Cc: Alexei Starovoitov <ast@plumgrid.com>
> Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: He Kuang <hekuang@huawei.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Kaixu Xia <xiakaixu@huawei.com>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Zefan Li <lizefan@huawei.com>
> Cc: pi3orama@163.com
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Link: http://lkml.kernel.org/n/ebpf-6yw9eg0ej3l4jnqhinngkw86@git.kernel.org
> ---
>  tools/lib/bpf/libbpf.c | 143 +++++++++++++++++++++++++++++++++++++++++++++----
>  tools/lib/bpf/libbpf.h |  22 ++++++++
>  2 files changed, 156 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 4252fc2..6a07b26 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -98,7 +98,11 @@ struct bpf_program {
>  	} *reloc_desc;
>  	int nr_reloc;
>  
> -	int fd;
> +	struct {
> +		int nr;
> +		int *fds;
> +	} instance;
> +	bpf_program_prep_t preprocessor;
>  
>  	struct bpf_object *obj;
>  	void *priv;
> @@ -152,10 +156,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->instance.nr == -1.
> +	 */
> +	if (prog->instance.nr > 0) {
> +		for (i = 0; i < prog->instance.nr; i++)
> +			zclose(prog->instance.fds[i]);
> +	} else if (prog->instance.nr != -1)
> +		pr_warning("Internal error: instance.nr is %d\n",
> +			   prog->instance.nr);
> +
> +	prog->instance.nr = -1;
> +	zfree(&prog->instance.fds);
>  }
>  
>  static void bpf_program__exit(struct bpf_program *prog)
> @@ -206,7 +224,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->instance.fds = NULL;
> +	prog->instance.nr = -1;
>  
>  	return 0;
>  errout:
> @@ -795,13 +814,71 @@ static int
>  bpf_program__load(struct bpf_program *prog,
>  		  char *license, u32 kern_version)
>  {
> -	int err, fd;
> +	int err = 0, fd, i;
> +
> +	if (prog->instance.nr < 0 || !prog->instance.fds) {
> +		if (prog->preprocessor) {
> +			pr_warning("Internal error: can't load program '%s'\n",
> +				   prog->section_name);
> +			return -EINVAL;
> +		}
> +
> +		prog->instance.fds = malloc(sizeof(int));
> +		if (!prog->instance.fds) {
> +			pr_warning("No enough memory for fds\n");
> +			return -ENOMEM;
> +		}
> +		prog->instance.nr = 1;
> +		prog->instance.fds[0] = -1;
> +	}
> +
> +	if (!prog->preprocessor) {
> +		if (prog->instance.nr != 1)
> +			pr_warning("Program '%s' inconsistent: nr(%d) not 1\n",
> +				   prog->section_name, prog->instance.nr);
>  
> -	err = load_program(prog->insns, prog->insns_cnt,
> -			   license, kern_version, &fd);
> -	if (!err)
> -		prog->fd = fd;
> +		err = load_program(prog->insns, prog->insns_cnt,
> +				   license, kern_version, &fd);
> +		if (!err)
> +			prog->instance.fds[0] = fd;
> +		goto out;
> +	}
> +
> +	for (i = 0; i < prog->instance.nr; i++) {
> +		struct bpf_prog_prep_result result;
> +		bpf_program_prep_t preprocessor = prog->preprocessor;
> +
> +		bzero(&result, sizeof(result));
> +		err = preprocessor(prog, i, prog->insns,
> +				   prog->insns_cnt, &result);
> +		if (err) {
> +			pr_warning("Preprocessing %dth instance of program '%s' failed\n",
> +					i, prog->section_name);
> +			goto out;
> +		}
> +
> +		if (!result.new_insn_ptr || !result.new_insn_cnt) {
> +			pr_debug("Skip loading %dth instance of program '%s'\n",
> +					i, prog->section_name);
> +			prog->instance.fds[i] = -1;
> +			continue;
> +		}
> +
> +		err = load_program(result.new_insn_ptr,
> +				   result.new_insn_cnt,
> +				   license, kern_version, &fd);
> +
> +		if (err) {
> +			pr_warning("Loading %dth instance of program '%s' failed\n",
> +					i, prog->section_name);
> +			goto out;
> +		}
>  
> +		if (result.pfd)
> +			*result.pfd = fd;
> +		prog->instance.fds[i] = fd;
> +	}
> +out:
>  	if (err)
>  		pr_warning("failed to load program '%s'\n",
>  			   prog->section_name);
> @@ -1052,5 +1129,53 @@ const char *bpf_program__title(struct bpf_program *prog, bool dup)
>  
>  int bpf_program__fd(struct bpf_program *prog)
>  {
> -	return prog->fd;
> +	return bpf_program__nth_fd(prog, 0);
> +}
> +
> +int bpf_program__set_prep(struct bpf_program *prog, int nr_instance,
> +			  bpf_program_prep_t prep)
> +{
> +	int *instance_fds;
> +
> +	if (nr_instance <= 0 || !prep)
> +		return -EINVAL;
> +
> +	if (prog->instance.nr > 0 || prog->instance.fds) {
> +		pr_warning("Can't set pre-processor after loading\n");
> +		return -EINVAL;
> +	}
> +
> +	instance_fds = malloc(sizeof(int) * nr_instance);
> +	if (!instance_fds) {
> +		pr_warning("alloc memory failed for instance of fds\n");
> +		return -ENOMEM;
> +	}
> +
> +	/* fill all fd with -1 */
> +	memset(instance_fds, 0xff, sizeof(int) * nr_instance);
> +
> +	prog->instance.nr = nr_instance;
> +	prog->instance.fds = instance_fds;
> +	prog->preprocessor = prep;
> +	return 0;
> +}
> +
> +int bpf_program__nth_fd(struct bpf_program *prog, int n)
> +{
> +	int fd;
> +
> +	if (n >= prog->instance.nr || n < 0) {
> +		pr_warning("Can't get the %dth fd from program %s: only %d instances\n",
> +			   n, prog->section_name, prog->instance.nr);
> +		return -EINVAL;
> +	}
> +
> +	fd = prog->instance.fds[n];
> +	if (fd < 0) {
> +		pr_warning("%dth instance of program '%s' is invalid\n",
> +			   n, prog->section_name);
> +		return -ENOENT;
> +	}
> +
> +	return fd;
>  }
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index f16170c..d82b89e 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -67,6 +67,28 @@ const char *bpf_program__title(struct bpf_program *prog, bool dup);
>  
>  int bpf_program__fd(struct bpf_program *prog);
>  
> +struct bpf_insn;
> +struct bpf_prog_prep_result {
> +	/*
> +	 * If not NULL, load new instruction array.
> +	 * If set to NULL, don't load this instance.
> +	 */
> +	struct bpf_insn *new_insn_ptr;
> +	int new_insn_cnt;
> +
> +	/* If not NULL, result fd is set to it */
> +	int *pfd;
> +};
> +
> +typedef int (*bpf_program_prep_t)(struct bpf_program *, int n,
> +				  struct bpf_insn *, int insn_cnt,
> +				  struct bpf_prog_prep_result *res);
> +
> +int bpf_program__set_prep(struct bpf_program *prog, int nr_instance,
> +			  bpf_program_prep_t prep);
> +
> +int bpf_program__nth_fd(struct bpf_program *prog, int n);
> +
>  /*
>   * We don't need __attribute__((packed)) now since it is
>   * unnecessary for 'bpf_map_def' because they are all aligned.
> -- 
> 1.8.3.4