netdev - Re: [PATCH bpf-next v2 6/7] libbpf: add 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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEf4BzaPewpUkPMvKXupQdANHbuAoEokC5g8eiugQcaJ_saVDg@mail.gmail.com>
Date:   Mon, 7 Jun 2021 16:41:33 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Kumar Kartikeya Dwivedi <memxor@...il.com>
Cc:     bpf <bpf@...r.kernel.org>,
        Toke Høiland-Jørgensen <toke@...hat.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Vlad Buslov <vladbu@...dia.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Networking <netdev@...r.kernel.org>
Subject: Re: [PATCH bpf-next v2 6/7] libbpf: add bpf_link based TC-BPF
 management API

On Thu, Jun 3, 2021 at 11:32 PM Kumar Kartikeya Dwivedi
<memxor@...il.com> wrote:
>
> This adds userspace TC-BPF management API based on bpf_link.
>
> Reviewed-by: Toke Høiland-Jørgensen <toke@...hat.com>.
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@...il.com>
> ---
>  tools/lib/bpf/bpf.c      |  8 +++++-
>  tools/lib/bpf/bpf.h      |  8 +++++-
>  tools/lib/bpf/libbpf.c   | 59 ++++++++++++++++++++++++++++++++++++++--
>  tools/lib/bpf/libbpf.h   | 17 ++++++++++++
>  tools/lib/bpf/libbpf.map |  1 +
>  tools/lib/bpf/netlink.c  |  5 ++--
>  tools/lib/bpf/netlink.h  |  8 ++++++
>  7 files changed, 100 insertions(+), 6 deletions(-)
>  create mode 100644 tools/lib/bpf/netlink.h
>

[...]

> +       link = calloc(1, sizeof(*link));
> +       if (!link)
> +               return ERR_PTR(-ENOMEM);
> +       link->detach = &bpf_link__detach_fd;
> +
> +       link_fd = bpf_link_create(prog_fd, OPTS_GET(hook, ifindex, 0), BPF_TC, &lopts);
> +       if (link_fd < 0) {
> +               link_fd = -errno;
> +               free(link);
> +               pr_warn("prog '%s': failed to attach tc filter: %s\n",
> +                       prog->name, libbpf_strerror_r(link_fd, errmsg, sizeof(errmsg)));
> +               return ERR_PTR(link_fd);

like Yonghong mentioned, all these error returns have to be either
`return libbpf_err_ptr(err);` or, given it's a new API, we can do just
direct `return errno = err, NULL;`. I'd probably use libbpf_err_ptr()
for now for consistency.

> +       }
> +       link->fd = link_fd;
> +
> +       return link;
> +}
> +
>  struct bpf_link *bpf_program__attach_freplace(struct bpf_program *prog,
>                                               int target_fd,
>                                               const char *attach_func_name)

[...]

> diff --git a/tools/lib/bpf/netlink.h b/tools/lib/bpf/netlink.h
> new file mode 100644
> index 000000000000..c89133d56eb4
> --- /dev/null
> +++ b/tools/lib/bpf/netlink.h
> @@ -0,0 +1,8 @@
> +// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
> +#pragma once

we don't use "#pragma once" in libbpf. And I saw some kernel
discussions discouraging its use. So please just stick to the classic
#ifdef/#define/#endif combo.

> +
> +#include <linux/types.h>
> +#include "libbpf.h"
> +
> +int tc_get_tcm_parent(enum bpf_tc_attach_point attach_point,
> +                     __u32 *parent);
> --
> 2.31.1
>