netdev - Re: [PATCH bpf-next 5/8] bpf: net: refactor bpf

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: <CAEf4BzYVcZkhthJAPW6QnLWGwznWpqAhOuTJtTVLMuNs6t0Zwg@mail.gmail.com>
Date:   Tue, 12 May 2020 15:19:55 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Yonghong Song <yhs@...com>
Cc:     Andrii Nakryiko <andriin@...com>, bpf <bpf@...r.kernel.org>,
        Martin KaFai Lau <kafai@...com>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <kernel-team@...com>
Subject: Re: [PATCH bpf-next 5/8] bpf: net: refactor bpf_iter target registration

On Tue, May 12, 2020 at 8:56 AM Yonghong Song <yhs@...com> wrote:
>
> Currently bpf_iter_reg_target takes parameters from target
> and allocates memory to save them. This is really not
> necessary, esp. in the future we may grow information
> passed from targets to bpf_iter manager.
>
> The patch refactors the code so target reg_info
> becomes static and bpf_iter manager can just take
> a reference to it.
>
> Signed-off-by: Yonghong Song <yhs@...com>
> ---
>  kernel/bpf/bpf_iter.c    | 29 +++++++++++------------------
>  kernel/bpf/map_iter.c    | 18 +++++++++---------
>  kernel/bpf/task_iter.c   | 30 ++++++++++++++++--------------
>  net/ipv6/route.c         | 18 +++++++++---------
>  net/netlink/af_netlink.c | 18 +++++++++---------
>  5 files changed, 54 insertions(+), 59 deletions(-)
>
> diff --git a/kernel/bpf/bpf_iter.c b/kernel/bpf/bpf_iter.c
> index b0c8b3bdf3b0..1d203dc7afe2 100644
> --- a/kernel/bpf/bpf_iter.c
> +++ b/kernel/bpf/bpf_iter.c
> @@ -8,11 +8,7 @@
>
>  struct bpf_iter_target_info {
>         struct list_head list;
> -       const char *target;
> -       const struct seq_operations *seq_ops;
> -       bpf_iter_init_seq_priv_t init_seq_private;
> -       bpf_iter_fini_seq_priv_t fini_seq_private;
> -       u32 seq_priv_size;
> +       struct bpf_iter_reg *reg_info;
>         u32 btf_id;     /* cached value */
>  };
>
> @@ -224,8 +220,8 @@ static int iter_release(struct inode *inode, struct file *file)
>         iter_priv = container_of(seq->private, struct bpf_iter_priv_data,
>                                  target_private);
>
> -       if (iter_priv->tinfo->fini_seq_private)
> -               iter_priv->tinfo->fini_seq_private(seq->private);
> +       if (iter_priv->tinfo->reg_info->fini_seq_private)
> +               iter_priv->tinfo->reg_info->fini_seq_private(seq->private);
>
>         bpf_prog_put(iter_priv->prog);
>         seq->private = iter_priv;
> @@ -248,11 +244,7 @@ int bpf_iter_reg_target(struct bpf_iter_reg *reg_info)

const struct bpf_iter_reg *? Can you please also add a comment that
passed struct is supposed to be static variable and live forever (so
not a dynamically allocated nor a stack variable)?

Also all the static struct bpf_iter_reg below should be marked const?

[...]