| 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
| ||
|
Date: Wed, 20 Apr 2016 16:20:37 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Wang Nan <wangnan0@...wei.com>
Cc: jolsa@...hat.com, brendan.d.gregg@...il.com,
linux-kernel@...r.kernel.org, pi3orama@....com,
Arnaldo Carvalho de Melo <acme@...hat.com>,
Alexei Starovoitov <ast@...nel.org>,
Jiri Olsa <jolsa@...nel.org>, Li Zefan <lizefan@...wei.com>
Subject: Re: [RFC PATCH 04/13] bpf tools: Replace fd array to union array
Em Wed, Apr 20, 2016 at 06:01:44PM +0000, Wang Nan escreveu:
> Following commits will add new types for instance of a bpf program.
> For kernel bpf program, a program instance is fd. For ubpf program,
> the instance of a program is a 'struct ubpf_vm'. This patch promote
> original 'fds' array to 'union prog_instance' array, prepare for
> further extending.
>
> Signed-off-by: Wang Nan <wangnan0@...wei.com>
> Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
> Cc: Alexei Starovoitov <ast@...nel.org>
> Cc: Brendan Gregg <brendan.d.gregg@...il.com>
> Cc: Jiri Olsa <jolsa@...nel.org>
> Cc: Li Zefan <lizefan@...wei.com>
> ---
> tools/lib/bpf/libbpf.c | 44 ++++++++++++++++++++++++--------------------
> 1 file changed, 24 insertions(+), 20 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 55b739e..dd5a107 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -138,6 +138,10 @@ int libbpf_strerror(int err, char *buf, size_t size)
> # define LIBBPF_ELF_C_READ_MMAP ELF_C_READ
> #endif
>
> +union prog_instance {
> + int fd;
> +};
> +
> /*
> * bpf_prog should be a better name but it has been used in
> * linux/filter.h.
> @@ -157,7 +161,7 @@ struct bpf_program {
>
> struct {
> int nr;
> - int *fds;
> + union prog_instance *array;
> } instances;
Why not just do:
struct {
int nr;
union {
int *fds;
};
} instances;
And then do no further changes and when going to the ubpf (haven't
looked at the actual patch, proposing this based on your above
description), just transform it into:
struct {
int nr;
union {
int *fds;
struct ubpf_vm *vms;
};
} instances;
Or plain use a void pointer and use:
struct {
int nr;
void *entries;
} instances;
if (prog->instances.nr > 0) {
int *fds = prog->instances.entries;
for (i = 0; i < prog->instances.nr; i++)
zclose(fd[i]);
}
> bpf_program_prep_t preprocessor;
>
> @@ -229,14 +233,14 @@ static void bpf_program__unload(struct bpf_program *prog)
> */
> if (prog->instances.nr > 0) {
> for (i = 0; i < prog->instances.nr; i++)
> - zclose(prog->instances.fds[i]);
> + zclose(prog->instances.array[i].fd);
> } else if (prog->instances.nr != -1) {
> pr_warning("Internal error: instances.nr is %d\n",
> prog->instances.nr);
> }
>
> prog->instances.nr = -1;
> - zfree(&prog->instances.fds);
> + zfree(&prog->instances.array);
> }
>
> static void bpf_program__exit(struct bpf_program *prog)
> @@ -287,7 +291,7 @@ 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->instances.fds = NULL;
> + prog->instances.array = NULL;
> prog->instances.nr = -1;
>
> return 0;
> @@ -937,20 +941,20 @@ bpf_program__load(struct bpf_program *prog,
> {
> int err = 0, fd, i;
>
> - if (prog->instances.nr < 0 || !prog->instances.fds) {
> + if (prog->instances.nr < 0 || !prog->instances.array) {
> if (prog->preprocessor) {
> pr_warning("Internal error: can't load program '%s'\n",
> prog->section_name);
> return -LIBBPF_ERRNO__INTERNAL;
> }
>
> - prog->instances.fds = malloc(sizeof(int));
> - if (!prog->instances.fds) {
> - pr_warning("Not enough memory for BPF fds\n");
> + prog->instances.array = malloc(sizeof(union prog_instance));
> + if (!prog->instances.array) {
> + pr_warning("Not enough memory for BPF instance\n");
> return -ENOMEM;
> }
> prog->instances.nr = 1;
> - prog->instances.fds[0] = -1;
> + prog->instances.array[0].fd = -1;
> }
>
> if (!prog->preprocessor) {
> @@ -961,7 +965,7 @@ bpf_program__load(struct bpf_program *prog,
> err = load_program(prog->insns, prog->insns_cnt,
> license, kern_version, &fd);
> if (!err)
> - prog->instances.fds[0] = fd;
> + prog->instances.array[0].fd = fd;
> goto out;
> }
>
> @@ -981,7 +985,7 @@ bpf_program__load(struct bpf_program *prog,
> if (!result.new_insn_ptr || !result.new_insn_cnt) {
> pr_debug("Skip loading the %dth instance of program '%s'\n",
> i, prog->section_name);
> - prog->instances.fds[i] = -1;
> + prog->instances.array[i].fd = -1;
> if (result.pfd)
> *result.pfd = -1;
> continue;
> @@ -999,7 +1003,7 @@ bpf_program__load(struct bpf_program *prog,
>
> if (result.pfd)
> *result.pfd = fd;
> - prog->instances.fds[i] = fd;
> + prog->instances.array[i].fd = fd;
> }
> out:
> if (err)
> @@ -1269,27 +1273,27 @@ int bpf_program__fd(struct bpf_program *prog)
> int bpf_program__set_prep(struct bpf_program *prog, int nr_instances,
> bpf_program_prep_t prep)
> {
> - int *instances_fds;
> + union prog_instance *array;
>
> if (nr_instances <= 0 || !prep)
> return -EINVAL;
>
> - if (prog->instances.nr > 0 || prog->instances.fds) {
> + if (prog->instances.nr > 0 || prog->instances.array) {
> pr_warning("Can't set pre-processor after loading\n");
> return -EINVAL;
> }
>
> - instances_fds = malloc(sizeof(int) * nr_instances);
> - if (!instances_fds) {
> - pr_warning("alloc memory failed for fds\n");
> + array = malloc(sizeof(array[0]) * nr_instances);
> + if (!array) {
> + pr_warning("alloc memory failed for instances\n");
> return -ENOMEM;
> }
>
> /* fill all fd with -1 */
> - memset(instances_fds, -1, sizeof(int) * nr_instances);
> + memset(array, -1, sizeof(array[0]) * nr_instances);
>
> prog->instances.nr = nr_instances;
> - prog->instances.fds = instances_fds;
> + prog->instances.array = array;
> prog->preprocessor = prep;
> return 0;
> }
> @@ -1304,7 +1308,7 @@ int bpf_program__nth_fd(struct bpf_program *prog, int n)
> return -EINVAL;
> }
>
> - fd = prog->instances.fds[n];
> + fd = prog->instances.array[n].fd;
> if (fd < 0) {
> pr_warning("%dth instance of program '%s' is invalid\n",
> n, prog->section_name);
> --
> 1.8.3.4
Powered by blists - more mailing lists