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

["Wangnan (F)" <wangnan0@huawei.com>]

On 2015/7/9 20:35, Wang Nan wrote:
> 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.
>
> 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
> preprocessor function which will be called when doing real loading.
> The callback should return instructions arrays for each instances.
>
> fd field in bpf_programs is replaced by instance, which has an nr field
> 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>
> ---
>   tools/lib/bpf/libbpf.c | 138 +++++++++++++++++++++++++++++++++++++++++++++----
>   tools/lib/bpf/libbpf.h |  22 ++++++++
>   2 files changed, 151 insertions(+), 9 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index b37aab1..6fcd042 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,19 @@ struct bpf_object {
>   
>   static void bpf_program__unload(struct bpf_program *prog)
>   {
> +	int i;
> +
>   	if (!prog)
>   		return;
>   
> -	zclose(prog->fd);
> +	if (prog->instance.nr < 0)
> +		pr_warning("Internal error when unloading: instance is %d\n",
> +			   prog->instance.nr);
> +	else
> +		for (i = 0; i < prog->instance.nr; i++)
> +			zclose(prog->instance.fds[i]);
> +	prog->instance.nr = -1;
> +	zfree(&prog->instance.fds);
>   }
>   
>   static void bpf_program__clear(struct bpf_program *prog)
> @@ -206,7 +219,8 @@ __bpf_program__new(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:
> @@ -798,13 +812,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;
> +		}
>   
> -	err = load_program(prog->insns, prog->insns_cnt,
> -			   license, kern_version, &fd);
> -	if (!err)
> -		prog->fd = fd;
> +		prog->instance.fds = malloc(sizeof(int));
> +		if (prog->instance.fds) {

I lost a '!' here. Should be:

+ if (!prog->instance.fds) {
...

My github tree has updated. Please check:

   https://github.com/WangNan0/linux.git perf/ebpf-for-acme


> +			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->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);
> @@ -1036,5 +1108,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 ea8adc2..9fa7b09 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -65,6 +65,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.